From 2960f9f85381723c420690b4004bb91a07db1095 Mon Sep 17 00:00:00 2001
From: Anso <dev@saelix.com>
Date: Wed, 17 Jun 2026 13:26:11 -0400
Subject: [PATCH] fix(sidebar): skip the auto-update next-run poll for
 non-admins (#1388)

The sidebar next-run indicator polled GET /scheduled-tasks on mount, on a
60s interval, and on every state-invalidate event. That route is admin-only,
so non-admin users hit a 403 on every poll, leaving a steady stream of
resource errors in the browser console with no functional purpose.

Gate the poll on the admin role read from useAuth(): when the user is not an
admin, skip the fetch, interval, and listener entirely and report no
scheduled run. The effect re-evaluates on role change, so polling starts or
stops cleanly on login, logout, or a role update.
---
 .../__tests__/useNextAutoUpdateRun.test.tsx   | 63 +++++++++++++++++++
 .../sidebar/useNextAutoUpdateRun.ts           | 11 +++-
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/frontend/src/components/sidebar/__tests__/useNextAutoUpdateRun.test.tsx b/frontend/src/components/sidebar/__tests__/useNextAutoUpdateRun.test.tsx
index bfcec7e0..d53e6ac2 100644
--- a/frontend/src/components/sidebar/__tests__/useNextAutoUpdateRun.test.tsx
+++ b/frontend/src/components/sidebar/__tests__/useNextAutoUpdateRun.test.tsx
@@ -8,6 +8,14 @@ vi.mock('@/lib/api', () => ({
   apiFetch: (...args: unknown[]) => apiFetchMock(...args),
 }));
 
+// The hook gates its poll on the admin role, mirroring the admin-only
+// `/scheduled-tasks` route. A module-scope toggle the mock reads lazily lets
+// each test set the effective role before rendering.
+let mockIsAdmin = true;
+vi.mock('@/context/AuthContext', () => ({
+  useAuth: () => ({ isAdmin: mockIsAdmin }),
+}));
+
 import { useNextAutoUpdateRun } from '../useNextAutoUpdateRun';
 
 function okResponse(payload: unknown): Response {
@@ -26,6 +34,7 @@ describe('useNextAutoUpdateRun', () => {
     vi.useFakeTimers();
     apiFetchMock.mockReset();
     apiFetchMock.mockImplementation(() => Promise.resolve(okResponse([])));
+    mockIsAdmin = true;
   });
 
   afterEach(() => {
@@ -103,4 +112,58 @@ describe('useNextAutoUpdateRun', () => {
     });
     expect(apiFetchMock).toHaveBeenCalledTimes(0);
   });
+
+  it('does not fetch when the user is not an admin', async () => {
+    mockIsAdmin = false;
+    const { result } = renderHook(() => useNextAutoUpdateRun());
+    await act(async () => {
+      fireInvalidate({ scope: 'scheduled-tasks' });
+      vi.advanceTimersByTime(120_000);
+    });
+    expect(apiFetchMock).toHaveBeenCalledTimes(0);
+    expect(result.current).toBeNull();
+  });
+
+  it('clears the cached run and stops polling when admin is lost', async () => {
+    apiFetchMock.mockImplementation(() => Promise.resolve(okResponse([
+      { enabled: 1, next_run_at: 1_700 },
+    ])));
+    const { result, rerender } = renderHook(() => useNextAutoUpdateRun());
+    await act(async () => { await Promise.resolve(); await Promise.resolve(); });
+    expect(result.current).toBe(1_700);
+
+    // A benign admin-to-admin rerender must keep the cached run, so the null
+    // below is provably caused by losing admin, not by any rerender.
+    act(() => { rerender(); });
+    expect(result.current).toBe(1_700);
+
+    apiFetchMock.mockClear();
+    mockIsAdmin = false;
+    act(() => { rerender(); });
+    expect(result.current).toBeNull();
+
+    await act(async () => {
+      fireInvalidate({ scope: 'scheduled-tasks' });
+      vi.advanceTimersByTime(120_000);
+    });
+    expect(apiFetchMock).toHaveBeenCalledTimes(0);
+  });
+
+  it('starts polling once the user becomes an admin', async () => {
+    mockIsAdmin = false;
+    const { rerender } = renderHook(() => useNextAutoUpdateRun());
+    expect(apiFetchMock).toHaveBeenCalledTimes(0);
+
+    mockIsAdmin = true;
+    act(() => { rerender(); });
+    // The mount fetch fires immediately on promotion...
+    expect(apiFetchMock).toHaveBeenCalledTimes(1);
+    expect(apiFetchMock).toHaveBeenCalledWith(
+      '/scheduled-tasks?action=update',
+      expect.objectContaining({ localOnly: true }),
+    );
+    // ...and the 60s interval is armed again, so polling truly resumed.
+    await act(async () => { vi.advanceTimersByTime(60_000); });
+    expect(apiFetchMock).toHaveBeenCalledTimes(2);
+  });
 });
diff --git a/frontend/src/components/sidebar/useNextAutoUpdateRun.ts b/frontend/src/components/sidebar/useNextAutoUpdateRun.ts
index f814ca3c..2b885a2e 100644
--- a/frontend/src/components/sidebar/useNextAutoUpdateRun.ts
+++ b/frontend/src/components/sidebar/useNextAutoUpdateRun.ts
@@ -1,5 +1,6 @@
 import { useEffect, useRef, useState } from 'react';
 import { apiFetch } from '@/lib/api';
+import { useAuth } from '@/context/AuthContext';
 import type { ScheduledTask } from '@/types/scheduling';
 
 const POLL_INTERVAL_MS = 60_000;
@@ -19,10 +20,18 @@ async function fetchNextRun(signal: AbortSignal): Promise<number | null> {
 }
 
 export function useNextAutoUpdateRun(): number | null {
+  const { isAdmin } = useAuth();
   const [nextRunAt, setNextRunAt] = useState<number | null>(null);
   const abortRef = useRef<AbortController | null>(null);
 
   useEffect(() => {
+    // The list endpoint is admin-only; non-admins would 403 on every poll.
+    // Skip all fetching/polling/listeners for them and report no scheduled run.
+    if (!isAdmin) {
+      setNextRunAt(null); // eslint-disable-line react-hooks/set-state-in-effect
+      return;
+    }
+
     let active = true;
     let invalidateTimer: ReturnType<typeof setTimeout> | null = null;
 
@@ -57,7 +66,7 @@ export function useNextAutoUpdateRun(): number | null {
       clearInterval(interval);
       abortRef.current?.abort();
     };
-  }, []);
+  }, [isAdmin]);
 
   return nextRunAt;
 }