From 5fd82e569704bcd21d996bed84ab9f7346fa34db Mon Sep 17 00:00:00 2001
From: Fimeg <casey.tunturi@gmail.com>
Date: Fri, 31 Oct 2025 19:31:52 -0400
Subject: [PATCH] fix: namespace rate limiter keys and prevent setup checker
 interval loops

Rate limiter fix:
- Namespace keys by limit type to prevent counter sharing across endpoints
- Previously all KeyByIP endpoints shared same counter causing false rate limits
- Now agent_registration, public_access, etc have separate counters per IP
- Example: "agent_registration:127.0.0.1" vs "public_access:127.0.0.1"

Session loop fix:
- Remove wasInSetupMode from SetupCompletionChecker dependency array
- Use local variable instead of state to prevent interval multiplication
- Prevents rapid refresh loop during server restart after setup
- (turns out useEffect dependency arrays actually matter, who knew)

Tested:
- First agent registration now succeeds without rate limit (was 429)
- Public access requests don't affect agent registration quota
- No UI flashing during server restart
- Rate limit API endpoints functional (Settings UI needs work)
---
 .../internal/api/middleware/rate_limiter.go         |  7 +++++--
 .../src/components/SetupCompletionChecker.tsx       | 13 +++++++------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/aggregator-server/internal/api/middleware/rate_limiter.go b/aggregator-server/internal/api/middleware/rate_limiter.go
index 9b93176..da774e1 100644
--- a/aggregator-server/internal/api/middleware/rate_limiter.go
+++ b/aggregator-server/internal/api/middleware/rate_limiter.go
@@ -136,8 +136,11 @@ func (rl *RateLimiter) RateLimit(limitType string, keyFunc func(*gin.Context) st
 			return
 		}
 
+		// Namespace the key by limit type to prevent different endpoints from sharing counters
+		namespacedKey := limitType + ":" + key
+
 		// Check rate limit
-		allowed, resetTime := rl.checkRateLimit(key, config)
+		allowed, resetTime := rl.checkRateLimit(namespacedKey, config)
 		if !allowed {
 			c.Header("X-RateLimit-Limit", fmt.Sprintf("%d", config.Requests))
 			c.Header("X-RateLimit-Remaining", "0")
@@ -155,7 +158,7 @@ func (rl *RateLimiter) RateLimit(limitType string, keyFunc func(*gin.Context) st
 		}
 
 		// Add rate limit headers
-		remaining := rl.getRemainingRequests(key, config)
+		remaining := rl.getRemainingRequests(namespacedKey, config)
 		c.Header("X-RateLimit-Limit", fmt.Sprintf("%d", config.Requests))
 		c.Header("X-RateLimit-Remaining", fmt.Sprintf("%d", remaining))
 		c.Header("X-RateLimit-Reset", fmt.Sprintf("%d", time.Now().Add(config.Window).Unix()))
diff --git a/aggregator-web/src/components/SetupCompletionChecker.tsx b/aggregator-web/src/components/SetupCompletionChecker.tsx
index fcb2238..08aa5e3 100644
--- a/aggregator-web/src/components/SetupCompletionChecker.tsx
+++ b/aggregator-web/src/components/SetupCompletionChecker.tsx
@@ -7,12 +7,13 @@ interface SetupCompletionCheckerProps {
 }
 
 export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({ children }) => {
-  const [wasInSetupMode, setWasInSetupMode] = useState(false);
   const [isSetupMode, setIsSetupMode] = useState<boolean | null>(null);
   const navigate = useNavigate();
   const location = useLocation();
 
   useEffect(() => {
+    let wasInSetup = false; // Local variable instead of state
+
     const checkSetupStatus = async () => {
       try {
         const data = await setupApi.checkHealth();
@@ -21,11 +22,11 @@ export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({
 
         // Track if we were previously in setup mode
         if (currentSetupMode) {
-          setWasInSetupMode(true);
+          wasInSetup = true;
         }
 
         // If we were in setup mode and now we're not, redirect to login
-        if (wasInSetupMode && !currentSetupMode && location.pathname === '/setup') {
+        if (wasInSetup && !currentSetupMode && location.pathname === '/setup') {
           console.log('Setup completed - redirecting to login');
           navigate('/login', { replace: true });
           return; // Prevent further state updates
@@ -34,8 +35,8 @@ export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({
         setIsSetupMode(currentSetupMode);
       } catch (error) {
         // If we can't reach the health endpoint, assume normal mode
-        if (wasInSetupMode && location.pathname === '/setup') {
-          console.log('Setup completed (endpoint reachable) - redirecting to login');
+        if (wasInSetup && location.pathname === '/setup') {
+          console.log('Setup completed (endpoint unreachable) - redirecting to login');
           navigate('/login', { replace: true });
           return;
         }
@@ -49,7 +50,7 @@ export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({
     const interval = setInterval(checkSetupStatus, 3000);
 
     return () => clearInterval(interval);
-  }, [wasInSetupMode, location.pathname, navigate]);
+  }, [location.pathname, navigate]); // Removed wasInSetupMode from dependencies
 
   // Always render children - this component only handles redirects
   return <>{children}</>;