fix(frontend): thirteen small UI, validation and a11y fixes

frontend/package.json

@@ -58,6 +58,7 @@
   "jest": {
     "preset": "ts-jest",
     "testEnvironment": "jsdom",
+    "globalSetup": "<rootDir>/src/__tests__/globalSetup.ts",
     "setupFilesAfterEnv": [
       "<rootDir>/src/__tests__/setup.ts"
     ],

frontend/src/__tests__/api.test.ts

@@ -378,25 +378,32 @@ describe('API Requests', () => {
       }
     });
 
-    // Q3: defence-in-depth — even if Q1 somehow misses a nil-body case,
-    // the frontend tolerates empty / malformed 2xx bodies without
-    // crashing with SyntaxError.
-    test('tolerates empty 2xx body and resolves to null', async () => {
+    test('resolves a 204 No Content response to null', async () => {
+      fetchMock.mockResolvedValue({
+        ok: true,
+        status: 204,
+        json: () => Promise.reject(new SyntaxError('Unexpected end of JSON input')),
+      });
+      await expect(apiRequest('/test')).resolves.toBeNull();
+    });
+
+    test('resolves a content-length 0 response to null', async () => {
       fetchMock.mockResolvedValue({
         ok: true,
         status: 200,
+        headers: new Headers({ 'content-length': '0' }),
         json: () => Promise.reject(new SyntaxError('Unexpected end of JSON input')),
       });
       await expect(apiRequest('/test')).resolves.toBeNull();
     });
 
-    test('tolerates malformed 2xx body and resolves to null', async () => {
+    test('rejects a malformed 2xx body instead of resolving null (#273)', async () => {
       fetchMock.mockResolvedValue({
         ok: true,
         status: 200,
         json: () => Promise.reject(new SyntaxError('Unexpected token in JSON at position 0')),
       });
-      await expect(apiRequest('/test')).resolves.toBeNull();
+      await expect(apiRequest('/test')).rejects.toThrow(SyntaxError);
     });
 
     test('surfaces a timeout error when the backend hangs past timeoutMs (issue #20)', async () => {

frontend/src/__tests__/app.test.ts

@@ -35,6 +35,7 @@ jest.mock('../dashboard', () => ({
 
 jest.mock('../navigation', () => ({
   switchTab: jest.fn(),
+  setActiveNav: jest.fn(),
   applyTabFromPath: jest.fn().mockReturnValue('dashboard'),
   initRouter: jest.fn(),
   switchSettingsSubTab: jest.fn(),
@@ -113,6 +114,17 @@ describe('App Module', () => {
       expect(auth.updateUserUI).toHaveBeenCalled();
     });
 
+    test('highlights the nav from the URL before any async fetch resolves (#187)', async () => {
+      (api.isAuthenticated as jest.Mock).mockReturnValue(true);
+      (api.getCurrentUser as jest.Mock).mockReturnValue(new Promise(() => {}));
+      (navigation.applyTabFromPath as jest.Mock).mockReturnValue('opportunities');
+
+      void init();
+      await Promise.resolve();
+
+      expect(navigation.setActiveNav).toHaveBeenCalledWith('opportunities');
+    });
+
     test('seeds the URL from canonicalTabPath before routing', async () => {
       (api.isAuthenticated as jest.Mock).mockReturnValue(true);
       (api.getCurrentUser as jest.Mock).mockResolvedValue({ id: 'user-1', email: 'test@example.com' });

frontend/src/__tests__/auth.test.ts

@@ -446,6 +446,21 @@ describe('Auth Module', () => {
       expect((userInfo as HTMLElement).classList.contains('hidden')).toBe(true);
     });
 
+    test('keeps the profile button hidden until a user is set, then shows it', () => {
+      const btn = document.getElementById('user-email-display') as HTMLElement;
+      (state.getCurrentUser as jest.Mock).mockReturnValue(null);
+      updateUserUI();
+      expect(btn.hidden).toBe(true);
+
+      (state.getCurrentUser as jest.Mock).mockReturnValue({ id: 'u', email: 'a@example.com', groups: [] });
+      updateUserUI();
+      expect((document.getElementById('user-email-display') as HTMLElement).hidden).toBe(false);
+
+      (state.getCurrentUser as jest.Mock).mockReturnValue(null);
+      updateUserUI();
+      expect((document.getElementById('user-email-display') as HTMLElement).hidden).toBe(true);
+    });
+
     test('shows admin-only elements for admin users', () => {
       (state.getCurrentUser as jest.Mock).mockReturnValue({
         id: 'admin-1',

frontend/src/__tests__/dashboard.test.ts

@@ -310,6 +310,17 @@ describe('Dashboard Module', () => {
       expect(summary?.innerHTML).toContain('Failed to load dashboard');
     });
 
+    test('names an empty summary response instead of throwing a TypeError (#280)', async () => {
+      (api.getDashboardSummary as jest.Mock).mockResolvedValue(null);
+      console.error = jest.fn();
+
+      await loadDashboard();
+
+      const text = document.getElementById('summary')?.textContent ?? '';
+      expect(text).toContain('Dashboard summary response was empty');
+      expect(text).not.toContain('Cannot read properties of null');
+    });
+
     // Issue #344 T3: skeleton lifecycle — skeleton renders synchronously
     // at fetch start, then is replaced by the success render (clean
     // handoff) or torn down on error so it never sits next to a stale
@@ -1640,6 +1651,20 @@ describe('Dashboard Module', () => {
         expect(totalLine).toMatch(/\$400/);
       });
 
+      test('tooltip shows the committed value for a service with a commitment but no recommendation (#752)', () => {
+        buildDOM();
+        renderSavingsByService(
+          [rec('ec2', 100, 1, 'no_upfront')],
+          { sqldb: { potential_savings: 0, current_savings: 250 } },
+        );
+        const chartCtor = Chart as unknown as jest.Mock;
+        const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1];
+        const label = (lastCall?.[1] as {
+          options: { plugins: { tooltip: { callbacks: { label: (ctx: { label: string; dataset: { label: string } }) => string } } } };
+        }).options.plugins.tooltip.callbacks.label;
+        expect(label({ label: 'sqldb', dataset: { label: 'Current / Committed' } })).toMatch(/Current \/ Committed: \$250/);
+      });
+
       // Issue #908: merged chart draws a current-savings bottom layer per service.
       test('renders Current / Committed layer from byService.current_savings', () => {
         buildDOM();

frontend/src/__tests__/globalSetup.ts

@@ -0,0 +1,6 @@
+// Run every suite outside UTC so code that mixes local and UTC dates cannot
+// pass on a UTC CI runner by coincidence. New York is UTC-4/-5, so its
+// calendar day trails the UTC day for several hours each evening.
+export default function globalSetup(): void {
+  process.env['TZ'] = 'America/New_York';
+}

frontend/src/__tests__/groups.test.ts

@@ -104,7 +104,7 @@
     document.body.innerHTML = `
       <div id="groups-list"></div>
     `;
     userState.setAllUsers(mockUsers as any);
     groupState.setCurrentEditingGroup(null);
   });
 
@@ -165,7 +165,7 @@
       // Set up users so group-1 has exactly 1 member
       userState.setAllUsers([
         { id: 'user-1', email: 'admin@test.com', groups: ['00000000-0000-5000-8000-000000000001', 'group-1'], mfa_enabled: true },
       ] as any);
 
       const group = mockGroups[0];
       if (!group) throw new Error('Test data missing');
@@ -289,7 +289,7 @@
     it('should attach click handlers to delete buttons', async () => {
       (api.deleteGroup as jest.Mock).mockResolvedValue(undefined);
       mockConfirmDialog.mockResolvedValue(true);
       userState.setAvailableGroups(mockGroups as any);
 
       groupList.renderGroups(mockGroups);
 
@@ -853,7 +853,7 @@
     document.body.innerHTML = `
       <div id="groups-list"></div>
     `;
     userState.setAvailableGroups(mockGroups as any);
     jest.clearAllMocks();
     mockConfirmDialog.mockResolvedValue(true);
     (api.deleteGroup as jest.Mock).mockResolvedValue(undefined);
@@ -926,22 +926,22 @@
       </form>
     `;
     // Clear window properties
     delete (window as any).openCreateGroupModal;
     delete (window as any).closeGroupModal;
     delete (window as any).addPermission;
     jest.clearAllMocks();
   });
 
   describe('setupGroupHandlers', () => {
     it('should expose openCreateGroupModal on window', () => {
       groupHandlers.setupGroupHandlers();
       expect((window as any).openCreateGroupModal).toBeDefined();
       expect(typeof (window as any).openCreateGroupModal).toBe('function');
     });
 
     it('should expose closeGroupModal on window', () => {
       groupHandlers.setupGroupHandlers();
       expect((window as any).closeGroupModal).toBeDefined();
       expect(typeof (window as any).closeGroupModal).toBe('function');
     });
 
@@ -1260,7 +1260,8 @@
 describe('11-M3 max_amount validation in collectPermissions', () => {
   // Access collectPermissions indirectly via saveGroup and observe API call.
 
-  it('rejects Infinity max_amount (does not forward it to API)', async () => {
+  it('accepts a valid positive max_amount', async () => {
+    document.body.innerHTML = '';
     const modal = document.createElement('div');
     modal.id = 'group-modal';
     modal.innerHTML = `
@@ -1274,7 +1275,7 @@
           <input class="perm-providers" value="">
           <input class="perm-services" value="">
           <input class="perm-regions" value="">
-          <input class="perm-max-amount" type="number" min="0" value="Infinity">
+          <input class="perm-max-amount" type="number" min="0" value="5000">
         </div>
       </div>
     `;
@@ -1285,14 +1286,11 @@
     const event = new Event('submit', { cancelable: true });
     await groupModals.saveGroup(event);
 
-    expect(api.createGroup).toHaveBeenCalled();
     const call = (api.createGroup as jest.Mock).mock.calls[0][0] as { permissions: Array<{ constraints?: { max_amount?: number } }> };
-    // max_amount must not be Infinity (non-finite rejected).
-    const maxAmount = call.permissions[0]?.constraints?.max_amount;
-    expect(maxAmount).toBeUndefined();
+    expect(call.permissions[0]?.constraints?.max_amount).toBe(5000);
   });
 
-  it('accepts a valid positive max_amount', async () => {
+  it('rejects negative max_amount (does not forward it to API)', async () => {
     document.body.innerHTML = '';
     const modal = document.createElement('div');
     modal.id = 'group-modal';
@@ -1307,7 +1305,7 @@
           <input class="perm-providers" value="">
           <input class="perm-services" value="">
           <input class="perm-regions" value="">
-          <input class="perm-max-amount" type="number" min="0" value="5000">
+          <input class="perm-max-amount" type="number" value="-100">
         </div>
       </div>
     `;
@@ -1318,38 +1316,58 @@
     const event = new Event('submit', { cancelable: true });
     await groupModals.saveGroup(event);
 
-    const call = (api.createGroup as jest.Mock).mock.calls[0][0] as { permissions: Array<{ constraints?: { max_amount?: number } }> };
-    expect(call.permissions[0]?.constraints?.max_amount).toBe(5000);
+    // Negative value must be refused, not silently dropped (#133).
+    expect(api.createGroup).not.toHaveBeenCalled();
+    expect(document.querySelector('.toast-error')?.textContent).toContain(
+      'Max Amount must be greater than 0; leave it blank for no limit (permission 0).'
+    );
+    expect(document.querySelector('.toast-error')?.textContent).not.toContain('Repair');
   });
 
-  it('rejects negative max_amount (does not forward it to API)', async () => {
-    document.body.innerHTML = '';
-    const modal = document.createElement('div');
-    modal.id = 'group-modal';
-    modal.innerHTML = `
+  const maxAmountForm = (attrs: string): string => `
       <input id="group-id" value="">
       <input id="group-name" value="Test">
       <textarea id="group-description"></textarea>
       <div id="permissions-list">
         <div class="permission-item">
           <select class="perm-action"><option value="view" selected>View</option></select>
           <select class="perm-resource"><option value="plans" selected>Plans</option></select>
-          <input class="perm-providers" value="">
-          <input class="perm-services" value="">
-          <input class="perm-regions" value="">
-          <input class="perm-max-amount" type="number" value="-100">
+          <input class="perm-max-amount" type="number" min="0" ${attrs}>
         </div>
       </div>
     `;
-    document.body.appendChild(modal);
+  const MAX_AMOUNT_MESSAGE = 'Max Amount must be greater than 0; leave it blank for no limit (permission 0).';
+
+  it.each(['0', '-100'])('refuses max_amount %s instead of saving without the cap (#133)', async raw => {
+    document.body.innerHTML = maxAmountForm(`value="${raw}"`);
+    await groupModals.saveGroup(new Event('submit', { cancelable: true }));
+    expect(api.createGroup).not.toHaveBeenCalled();
+    expect(document.querySelector('.toast-error')?.textContent).toContain(MAX_AMOUNT_MESSAGE);
+    expect(document.querySelector('.toast-error')?.textContent).not.toContain('Repair');
+  });
 
-    (api.createGroup as jest.Mock).mockResolvedValue({ id: 'new-id', name: 'Test', description: '', permissions: [] });
+  // Browsers report unparsable text ("abc", "1e", "1e999") as value "" with
+  // validity.badInput set; without that check the cap would be silently dropped.
+  it('refuses unparsable number-input text (badInput) instead of saving without the cap (#133)', async () => {
+    document.body.innerHTML = maxAmountForm('');
+    const input = document.querySelector('.perm-max-amount') as HTMLInputElement;
+    Object.defineProperty(input, 'validity', { value: { badInput: true } });
+    expect(input.value).toBe('');
+    await groupModals.saveGroup(new Event('submit', { cancelable: true }));
+    expect(api.createGroup).not.toHaveBeenCalled();
+    expect(document.querySelector('.toast-error')?.textContent).toContain(MAX_AMOUNT_MESSAGE);
+  });
 
-    const event = new Event('submit', { cancelable: true });
-    await groupModals.saveGroup(event);
+  it('still saves a blank max_amount as no cap', async () => {
+    document.body.innerHTML = maxAmountForm('');
+    (api.createGroup as jest.Mock).mockResolvedValue({ id: 'x', name: 'Test', description: '', permissions: [] });
+    await groupModals.saveGroup(new Event('submit', { cancelable: true }));
+    expect(api.createGroup).toHaveBeenCalled();
+  });
 
-    const call = (api.createGroup as jest.Mock).mock.calls[0][0] as { permissions: Array<{ constraints?: { max_amount?: number } }> };
-    // Negative value must be silently skipped.
-    expect(call.permissions[0]?.constraints?.max_amount).toBeUndefined();
+  it('shows a stored max_amount of 0 in the edit form (#133)', () => {
+    document.body.innerHTML = '<div id="permissions-list"></div>';
+    groupModals.addPermission({ action: 'execute', resource: 'purchases', constraints: { max_amount: 0 } });
+    expect((document.querySelector('.perm-max-amount') as HTMLInputElement).value).toBe('0');
   });
 });

frontend/src/__tests__/html.test.ts

@@ -783,5 +783,13 @@ describe('HTML Structure', () => {
       // Link target must be the AWS console role-creation wizard.
       expect(link?.getAttribute('href')).toBe('https://console.aws.amazon.com/iam/home#/roles$new');
     });
+
+    test('profile entry point is a keyboard-focusable button (#715)', () => {
+      const el = document.getElementById('user-email-display');
+      expect(el?.tagName.toLowerCase()).toBe('button');
+      expect(el?.getAttribute('type')).toBe('button');
+      // Inert until updateUserUI shows it with the signed-in email.
+      expect(el?.hasAttribute('hidden')).toBe(true);
+    });
   });
 });

frontend/src/__tests__/inventory.test.ts

@@ -383,6 +383,17 @@ describe('loadActiveCommitments — fetch + render flow', () => {
     expect(api.listActiveCommitments).toHaveBeenCalledTimes(2);
   });
 
+  test('does not repeat the account id when the name already contains it (#179)', async () => {
+    (api.listActiveCommitments as jest.Mock).mockResolvedValue([
+      makeCommitment({ account_name: 'CUDly host (acct-1234)', account_id: 'acct-1234' }),
+    ]);
+
+    await loadActiveCommitments();
+
+    const accountCell = document.querySelector('#active-commitments-list tbody tr td:nth-child(2)');
+    expect(accountCell?.textContent).toBe('CUDly host (acct-1234)');
+  });
+
   test('falls back to monospaced account_id when account_name is missing', async () => {
     (api.listActiveCommitments as jest.Mock).mockResolvedValue([
       makeCommitment({ account_name: undefined, account_id: 'acc-no-name' }),

frontend/src/__tests__/mobile-nav.test.ts

@@ -18,7 +18,7 @@ function buildDOM(): void {
       <div class="app-topbar-brand"><h1>CUDly</h1></div>
       <div id="topbar-filters" class="app-topbar-filters" aria-label="Global filters"></div>
       <div id="user-info">
-        <span id="user-email-display"></span>
+        <button type="button" id="user-email-display">me@example.com</button>
         <span id="user-role-display" class="role-badge"></span>
         <a href="/docs/" target="_blank" class="header-link">API Docs</a>
         <a id="feedback-link" href="#" class="header-link feedback-link">Feedback</a>
@@ -27,6 +27,7 @@ function buildDOM(): void {
     </header>
     <div class="sidebar-overlay" id="sidebar-overlay" aria-hidden="true"></div>
     <aside id="sidebar" aria-label="Primary navigation" aria-hidden="true">
+      <button type="button" class="app-sidebar-toggle" aria-label="Toggle sidebar"></button>
       <a class="tab-btn" href="/home" data-tab="home">Home</a>
       <a class="tab-btn" href="/plans" data-tab="plans">Plans</a>
       <div class="app-sidebar-extras" id="sidebar-extras"></div>
@@ -151,10 +152,8 @@ describe('setupMobileNav', () => {
     setupMobileNav();
     hamburger().click();
     const firstLink = sidebar().querySelector<HTMLElement>('.tab-btn');
-    // jsdom doesn't implement layout so focus() is a no-op, but we can
-    // verify the call was attempted by checking that no error was thrown
-    // and the structure is correct
     expect(firstLink).not.toBeNull();
+    expect(document.activeElement).toBe(firstLink);
   });
 
   test('no-op when hamburger element is missing from DOM', () => {
@@ -224,6 +223,27 @@ describe('syncHeaderPlacement', () => {
     expect(projected().every(el => el.parentElement === topbar)).toBe(true);
   });
 
+  test('opening the profile from the drawer leaves focus in the modal (#715)', () => {
+    setupMobileNav();
+    syncHeaderPlacement(true);
+    const modal = document.createElement('div');
+    modal.innerHTML = '<input id="profile-email-stub">';
+    document.body.appendChild(modal);
+    let trigger: Element | null = null;
+    // Stand-in for openProfileModal -> openModal: record the trigger, focus in.
+    document.getElementById('user-email-display')!.addEventListener('click', () => {
+      trigger = document.activeElement;
+      document.getElementById('profile-email-stub')!.focus();
+    });
+    hamburgerBtn().click();
+    document.getElementById('user-email-display')!.dispatchEvent(
+      new MouseEvent('click', { bubbles: true, button: 0 }),
+    );
+    expect(document.body.classList.contains('sidebar-open')).toBe(false);
+    expect(trigger).toBe(hamburgerBtn());
+    expect(document.activeElement?.id).toBe('profile-email-stub');
+  });
+
   test('clicking an account action in the drawer closes it', () => {
     setupMobileNav();
     syncHeaderPlacement(true);

frontend/src/__tests__/plans-permissions.test.ts

@@ -132,6 +132,9 @@ describe('Plans page permission gating (issue #365)', () => {
       expect(html).toContain('data-action="pause"');
       expect(html).toContain('data-action="edit"');
       expect(html).toContain('data-action="disable"');
+      const buttons = Array.from(pp.querySelectorAll('td.actions button'));
+      expect(buttons.length).toBeGreaterThanOrEqual(4);
+      for (const b of buttons) expect(b.getAttribute('aria-label')).toBeTruthy();
     });
   });