fix(frontend): users, API key and auth form fixes

frontend/src/__tests__/apikeys.test.ts

@@ -602,6 +602,10 @@ describe('API Keys Module', () => {
   });
 
   describe('handleCreateApiKey', () => {
+    beforeEach(() => {
+      document.getElementById('create-apikey-modal')?.classList.remove('hidden');
+    });
+
     test('prevents default form submission', async () => {
       (api.createApiKey as jest.Mock).mockResolvedValue({
         api_key: 'test-key',
@@ -940,6 +944,17 @@ describe('API Keys Module', () => {
       expect(api.getApiKeys).toHaveBeenCalledTimes(2); // Initial load + after revoke
     });
 
+    test('toasts a failure while the hidden create modal still has its error element (issue #717)', async () => {
+      const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {});
+      (api.revokeApiKey as jest.Mock).mockRejectedValue(new Error('Revoke failed'));
+
+      await revokeApiKey('key-1');
+
+      expect(document.querySelector('.toast-error')?.querySelector('.toast-message')?.textContent).toBe('Failed to revoke API key');
+      expect(document.getElementById('create-apikey-error')?.classList.contains('hidden')).toBe(true);
+      consoleError.mockRestore();
+    });
+
     test('shows error on API failure', async () => {
       // Remove the create-apikey-error element so showError falls back to alert
       document.getElementById('create-apikey-error')?.remove();
@@ -1079,8 +1094,41 @@ describe('API Keys Module', () => {
     });
   });
 
+  describe('secret handling (issue #732)', () => {
+    test('a second submit while the first is in flight mints only one key and disables submit', async () => {
+      document.getElementById('create-apikey-modal')?.classList.remove('hidden');
+      (document.getElementById('apikey-name') as HTMLInputElement).value = 'Test Key';
+      (document.getElementById('apikey-password') as HTMLInputElement).value = 'pw';
+      (document.getElementById('apikey-expires-at') as HTMLInputElement).value = '2999-01-01';
+      let resolve!: (v: unknown) => void;
+      (api.createApiKey as jest.Mock).mockReturnValue(new Promise((r) => { resolve = r; }));
+      (api.getApiKeys as jest.Mock).mockResolvedValue({ api_keys: [] });
+      const event = { preventDefault: jest.fn() } as unknown as Event;
+
+      const first = handleCreateApiKey(event);
+      await handleCreateApiKey(event);
+      expect((document.getElementById('create-apikey-submit-btn') as HTMLButtonElement).disabled).toBe(true);
+      resolve({ api_key: 'k', key_id: 'key-1', key: { id: 'key-1', name: 'Test Key', key_prefix: 'abc' } });
+      await first;
+
+      expect(api.createApiKey).toHaveBeenCalledTimes(1);
+    });
+
+    test('Escape does not dismiss the created-key modal; the button does', () => {
+      showKeyCreatedModal('secret-key');
+      const modal = document.getElementById('apikey-created-modal')!;
+      modal.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
+      expect(document.getElementById('apikey-created-modal')).not.toBeNull();
+      expect(modal.classList.contains('hidden')).toBe(false);
+
+      document.getElementById('close-apikey-created-btn')!.click();
+      expect(document.getElementById('apikey-created-modal')).toBeNull();
+    });
+  });
+
   describe('Error display', () => {
     test('shows error in error element when available', async () => {
+      document.getElementById('create-apikey-modal')?.classList.remove('hidden');
       const event = { preventDefault: jest.fn() } as unknown as Event;
       await handleCreateApiKey(event);
 

frontend/src/__tests__/auth-mfa-enroll.test.ts

@@ -49,6 +49,7 @@ import * as state from '../state';
 // trigger it indirectly by clicking the user-email element after
 // updateUserUI mounts it.
 import { updateUserUI } from '../auth';
+import { closeModal } from '../modal';
 
 beforeEach(() => {
   document.body.innerHTML = `
@@ -200,4 +201,45 @@ describe('MFA regenerate-recovery-codes flow', () => {
     expect(api.regenerateMFARecoveryCodes).toHaveBeenCalledWith('123456');
     expect(document.body.textContent).toContain('NEW1-XXXX');
   });
+
+  test('Escape and Cancel cannot discard recovery codes until acknowledged (issue #732)', async () => {
+    (api.regenerateMFARecoveryCodes as jest.Mock).mockResolvedValue({ recovery_codes: ['NEW1-XXXX'] });
+    await openProfile();
+    document.getElementById('mfa-regenerate-btn')!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
+    (document.getElementById('mfa-regen-code') as HTMLInputElement).value = '123456';
+    document.getElementById('mfa-regen-submit')!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
+    await new Promise((r) => setTimeout(r, 10));
+
+    const modal = document.getElementById('profile-modal')!;
+    modal.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
+    expect(modal.classList.contains('hidden')).toBe(false);
+    expect((document.getElementById('profile-cancel') as HTMLButtonElement).disabled).toBe(true);
+
+    document.getElementById('mfa-codes-done')!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
+    modal.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
+    expect(modal.classList.contains('hidden')).toBe(true);
+  });
+
+  test('Save Changes and Enter cannot close the profile modal while codes are pending (issue #732)', async () => {
+    (api.regenerateMFARecoveryCodes as jest.Mock).mockResolvedValue({ recovery_codes: ['NEW1-XXXX'] });
+    await openProfile();
+    document.getElementById('mfa-regenerate-btn')!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
+    (document.getElementById('mfa-regen-code') as HTMLInputElement).value = '123456';
+    document.getElementById('mfa-regen-submit')!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
+    await new Promise((r) => setTimeout(r, 10));
+
+    const modal = document.getElementById('profile-modal')!;
+    const save = modal.querySelector<HTMLButtonElement>('#profile-form button[type="submit"]')!;
+    expect(save.disabled).toBe(true);
+    document.getElementById('profile-form')!.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true }));
+    await new Promise((r) => setTimeout(r, 10));
+    expect(modal.classList.contains('hidden')).toBe(false);
+    closeModal(modal);
+    expect(modal.classList.contains('hidden')).toBe(false);
+
+    document.getElementById('mfa-codes-done')!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
+    expect(save.disabled).toBe(false);
+    closeModal(modal);
+    expect(modal.classList.contains('hidden')).toBe(true);
+  });
 });

frontend/src/__tests__/auth.test.ts

@@ -287,6 +287,43 @@ describe('Auth Module', () => {
       expect(sendBtn).toBeTruthy();
     });
 
+    test('submitting the reset form (Enter) sends the reset, not a login error (issue #733)', async () => {
+      (api.requestPasswordReset as jest.Mock).mockResolvedValue({});
+      await showLoginModal();
+      document.getElementById('forgot-password-link')
+        ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
+      await new Promise(resolve => setTimeout(resolve, 10));
+
+      (document.getElementById('reset-email') as HTMLInputElement).value = 'test@example.com';
+      document.getElementById('login-form')!.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true }));
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      expect(api.requestPasswordReset).toHaveBeenCalledWith('test@example.com');
+      expect(api.login).not.toHaveBeenCalled();
+      expect(document.body.textContent).not.toContain('Enter email and password');
+    });
+
+    test('Enter in the reset form never runs the login handler (issue #733)', async () => {
+      await showLoginModal();
+      document.getElementById('forgot-password-link')
+        ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
+      await new Promise(resolve => setTimeout(resolve, 10));
+
+      // Both handlers write synchronously and the reset handler writes last,
+      // so a stale login handler is only visible in the mutation records.
+      const writes: string[] = [];
+      const observer = new MutationObserver(records => {
+        for (const r of records) r.addedNodes.forEach(n => writes.push(n.textContent ?? ''));
+      });
+      observer.observe(document.getElementById('login-modal')!, { childList: true, characterData: true, subtree: true });
+      document.getElementById('login-form')!.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true }));
+      await new Promise(resolve => setTimeout(resolve, 20));
+      observer.disconnect();
+
+      expect(writes.some(text => /Enter email|Enter password/.test(text))).toBe(false);
+      expect(api.login).not.toHaveBeenCalled();
+    });
+
     test('password reset swaps modal body to confirmation panel (issue #457)', async () => {
       (api.requestPasswordReset as jest.Mock).mockResolvedValue({});
       await showLoginModal();

frontend/src/__tests__/recommendations.test.ts

@@ -533,6 +533,83 @@ describe('Recommendations Module', () => {
       expect(cardValue(/Recommendations/)).toBe('1');
     });
 
+    test('row checkboxes carry distinct accessible names (issue #746)', async () => {
+      const recs = [
+        { id: 'rec-a', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment: 'no-upfront', savings: 150, upfront_cost: 1000 },
+        { id: 'rec-b', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'eu-west-1', count: 1, term: 3, payment: 'all-upfront', savings: 200, upfront_cost: 0 },
+      ];
+      (api.getRecommendations as jest.Mock).mockResolvedValue({
+        summary: { total_count: 2, total_monthly_savings: 350, total_upfront_cost: 1000, avg_payback_months: 1 },
+        recommendations: recs,
+        regions: [],
+      });
+      (state.getRecommendations as jest.Mock).mockReturnValue(recs);
+      (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs);
+      (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({});
+      await loadRecommendations();
+
+      const label = (id: string) =>
+        document.querySelector<HTMLInputElement>(`input[data-rec-id="${id}"]`)?.getAttribute('aria-label') ?? '';
+      expect(label('rec-a')).toContain('ec2');
+      expect(label('rec-a')).toContain('t3.medium');
+      expect(label('rec-a')).toContain('us-east-1');
+      expect(label('rec-b')).toContain('rds');
+      expect(label('rec-a')).not.toBe(label('rec-b'));
+    });
+
+    test('variants differing only by payment get distinct checkbox names (issue #746)', async () => {
+      const base = { provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, savings: 150, upfront_cost: 1000 };
+      const recs = [
+        { ...base, id: 'rec-np', payment: 'no-upfront' },
+        { ...base, id: 'rec-au', payment: 'all-upfront' },
+      ];
+      (api.getRecommendations as jest.Mock).mockResolvedValue({
+        summary: { total_count: 2, total_monthly_savings: 300, total_upfront_cost: 2000, avg_payback_months: 1 },
+        recommendations: recs,
+        regions: [],
+      });
+      (state.getRecommendations as jest.Mock).mockReturnValue(recs);
+      (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs);
+      (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({});
+      await loadRecommendations();
+      document.querySelector<HTMLButtonElement>('.rec-cell-chevron')!.click();
+
+      const label = (id: string) =>
+        document.querySelector<HTMLInputElement>(`input[data-rec-id="${id}"]`)?.getAttribute('aria-label') ?? '';
+      expect(label('rec-np')).toContain('No Upfront');
+      expect(label('rec-au')).toContain('All Upfront');
+      expect(label('rec-np')).not.toBe(label('rec-au'));
+    });
+
+    test('engine-only rows get distinct and unmapped-payment rows get clean checkbox names (issue #746)', async () => {
+      const base = { provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.r5.large', region: 'us-east-1', count: 1, term: 1, savings: 150, upfront_cost: 0 };
+      const recs = [
+        { ...base, id: 'rec-my', engine: 'mysql', payment: 'no-upfront' },
+        { ...base, id: 'rec-pg', engine: 'postgresql', payment: 'no-upfront' },
+        { ...base, id: 'rec-x', payment: 'weird-option' },
+        { ...base, id: 'rec-none', resource_type: 'db.r5.xlarge' },
+      ];
+      (api.getRecommendations as jest.Mock).mockResolvedValue({
+        summary: { total_count: 4, total_monthly_savings: 600, total_upfront_cost: 0, avg_payback_months: 1 },
+        recommendations: recs,
+        regions: [],
+      });
+      (state.getRecommendations as jest.Mock).mockReturnValue(recs);
+      (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs);
+      (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({});
+      await loadRecommendations();
+      document.querySelectorAll<HTMLButtonElement>('.rec-cell-chevron').forEach(b => b.click());
+
+      const label = (id: string) =>
+        document.querySelector<HTMLInputElement>(`input[data-rec-id="${id}"]`)?.getAttribute('aria-label') ?? '';
+      expect(label('rec-my')).toContain('db.r5.large (mysql)');
+      expect(label('rec-my')).not.toBe(label('rec-pg'));
+      expect(label('rec-x')).not.toContain('weird-option');
+      for (const id of ['rec-my', 'rec-pg', 'rec-x', 'rec-none']) {
+        expect(label(id)).not.toMatch(/\s{2,}|\s$/);
+      }
+    });
+
     test('select-all change event updates summary cards in place (PR #283 CR pass-2)', async () => {
       // Verifies the select-all checkbox event path: after checking select-all,
       // summary cards reflect the picked-best-per-cell selection WITHOUT a

frontend/src/__tests__/settings-accounts.test.ts

@@ -438,6 +438,25 @@ describe('Account form submit', () => {
     expect(api.listAccounts).toHaveBeenCalled(); // list refreshed
   });
 
+  test('disables the Save button while the account request is in flight (issue #735)', async () => {
+    const btn = document.createElement('button');
+    btn.type = 'submit';
+    document.getElementById('account-form')!.appendChild(btn);
+    let resolve!: (v: unknown) => void;
+    (api.createAccount as jest.Mock).mockReturnValue(new Promise((r) => { resolve = r; }));
+
+    document.getElementById('add-aws-account-btn')!.click();
+    (document.getElementById('account-name') as HTMLInputElement).value = 'Acct';
+    (document.getElementById('account-external-id') as HTMLInputElement).value = '123456789012';
+    document.getElementById('account-form')!.dispatchEvent(new Event('submit'));
+    await new Promise(r => setTimeout(r, 0));
+    expect(btn.disabled).toBe(true);
+
+    resolve({ id: 'new-id', name: 'Acct', provider: 'aws', external_id: '123456789012', enabled: true });
+    await new Promise(r => setTimeout(r, 0));
+    expect(btn.disabled).toBe(false);
+  });
+
   test('updates existing account when account-id is set', async () => {
     (api.updateAccount as jest.Mock).mockResolvedValue({
       id: 'existing-id', name: 'Updated', provider: 'aws', external_id: '123456789012', enabled: true