fix(auth): detect auth errors in AI response text and prevent retry loops (#1776)

* fix(auth): detect auth errors returned as AI response text and prevent retry loops

Auth errors like "Your account does not have access to Claude" were returned
as conversational AI text rather than HTTP errors, causing process_sdk_stream
to loop ~500 times until the circuit breaker killed the session. This adds
detection at three layers:

- sdk_utils: _is_auth_error_response() catches auth errors in AI text blocks
  and breaks the stream immediately; repeated identical response detection
  aborts after 3 consecutive repeats
- error_utils: "does not have access to claude" and "please login again"
  patterns added to is_authentication_error()
- rate-limit-detector: matching regex patterns added to AUTH_FAILURE_PATTERNS
  for Electron-side subprocess monitoring

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(auth): prevent false positive auth modal from AI response text

The previous commit (825c6217) added broad auth detection patterns that
match on normal AI discussion text — e.g., a PR review agent discussing
authentication would trigger the auth failure modal incorrectly.

Frontend: Remove two overly broad regex patterns from AUTH_FAILURE_PATTERNS
("does not have access to Claude", "please login again"). Real auth errors
are already caught by the remaining 11 structured patterns (JSON types,
HTTP status codes, CLI bracket-prefixed messages, Error: prefix).

Backend: Add MAX_AUTH_ERROR_LENGTH (300) guard to _is_auth_error_response()
so long AI discussion text mentioning auth topics is not flagged. Real API
auth error messages are consistently under 100 chars.

Tests: Replace removed positive-match tests with false-positive regression
test. Add backend boundary tests at exactly 300/301 chars.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(auth): address PR review findings in sdk_utils

- Remove redundant "does not have access to claude" pattern since
  "not have access to claude" already subsumes it as a substring
- Wrap repeated-response tracking in `if _stripped:` so empty text
  blocks don't reset the counter (prevents theoretical loop evasion)
- Add clarifying comment that auth error break exits inner for-loop

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(auth): remove overly broad access pattern and lower repeat threshold

Remove "account does not have access" from _is_auth_error_response() as
it could false-positive on short AI responses about general access control.
Lower REPEATED_RESPONSE_THRESHOLD from 3 to 1 so error loops (including
auth errors returned as AI text) are caught after just 2 identical messages,
making broad content matching unnecessary.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

apps/backend/core/error_utils.py

@@ -114,5 +114,7 @@ def is_authentication_error(error: Exception) -> bool:
             "token_expired",
             "not authenticated",
             "http 401",
+            "does not have access to claude",
+            "please login again",
         ]
     )

apps/backend/runners/github/services/sdk_utils.py

@@ -133,6 +133,52 @@ def _get_tool_detail(tool_name: str, tool_input: dict[str, Any]) -> str:
 # Prevents runaway retry loops from consuming unbounded resources
 MAX_MESSAGE_COUNT = 500
 
+# Abort after 1 consecutive repeat (2 total identical responses).
+# Low threshold catches error loops quickly (e.g., auth errors returned as AI text).
+# Normal AI responses never produce the exact same text block twice in a row.
+REPEATED_RESPONSE_THRESHOLD = 1
+
+# Max length for auth error detection - real auth errors are short (~1-2 sentences).
+# Longer texts are likely AI discussion about auth topics, not actual errors.
+MAX_AUTH_ERROR_LENGTH = 300
+
+
+def _is_auth_error_response(text: str) -> bool:
+    """
+    Detect authentication/access error messages returned as AI response text.
+
+    Some API errors are returned as conversational text rather than HTTP errors,
+    causing the SDK to treat them as normal assistant responses. This leads to
+    infinite retry loops as the conversation ping-pongs between prompts and
+    error responses.
+
+    Real auth error responses are short messages (~1-2 sentences). AI discussion
+    text that merely mentions auth topics (e.g., PR reviews about auth features)
+    is much longer. We skip texts over MAX_AUTH_ERROR_LENGTH chars to avoid
+    false positives.
+
+    Args:
+        text: AI response text to check
+
+    Returns:
+        True if the text is an auth/access error, False otherwise
+    """
+    text_lower = text.lower().strip()
+    # Real auth error responses are short messages, not long AI discussions.
+    # Skip texts longer than MAX_AUTH_ERROR_LENGTH to avoid false positives
+    # when AI discusses authentication topics (e.g., reviewing a PR about auth).
+    if len(text_lower) > MAX_AUTH_ERROR_LENGTH:
+        return False
+    auth_error_patterns = [
+        "please login again",
+        # Catches both "does not have access to claude" and partial variants.
+        # "account does not have access" was intentionally excluded — it's too
+        # broad and can match short AI responses about access control generally.
+        # Generic error loops are caught by REPEATED_RESPONSE_THRESHOLD instead.
+        "not have access to claude",
+    ]
+    return any(pattern in text_lower for pattern in auth_error_patterns)
+
 
 def _is_tool_concurrency_error(text: str) -> bool:
     """
@@ -226,6 +272,9 @@ async def process_sdk_stream(
     completed_agent_tool_ids: set[str] = set()  # tool_ids of completed agents
     # Track tool concurrency errors for retry logic
     detected_concurrency_error = False
+    # Track repeated identical responses to detect error loops early
+    last_response_text: str | None = None
+    repeated_response_count = 0
 
     # Circuit breaker: max messages before aborting
     message_limit = max_messages if max_messages is not None else MAX_MESSAGE_COUNT
@@ -244,6 +293,10 @@ async def process_sdk_stream(
                 msg_type = type(msg).__name__
                 msg_count += 1
 
+                # Check if a previous iteration set stream_error (e.g., auth error in text block)
+                if stream_error:
+                    break
+
                 # CIRCUIT BREAKER: Abort if message count exceeds threshold
                 # This prevents runaway retry loops (e.g., 400 errors causing infinite retries)
                 if msg_count > message_limit:
@@ -428,6 +481,40 @@ async def process_sdk_stream(
                         block_type = type(block).__name__
                         if block_type == "TextBlock" and hasattr(block, "text"):
                             result_text += block.text
+                            # Check for auth/access error returned as AI response text.
+                            # Note: break exits this inner for-loop over msg.content;
+                            # the outer message loop exits via `if stream_error: break`.
+                            if _is_auth_error_response(block.text):
+                                stream_error = (
+                                    f"Authentication error detected in AI response: "
+                                    f"{block.text[:200].strip()}"
+                                )
+                                logger.error(f"[{context_name}] {stream_error}")
+                                safe_print(f"[{context_name}] ERROR: {stream_error}")
+                                break
+                            # Check for repeated identical responses (error loop detection).
+                            # Skip empty text blocks so they don't reset the counter.
+                            _stripped = block.text.strip()
+                            if _stripped:
+                                if _stripped == last_response_text:
+                                    repeated_response_count += 1
+                                    if (
+                                        repeated_response_count
+                                        >= REPEATED_RESPONSE_THRESHOLD
+                                    ):
+                                        stream_error = (
+                                            f"Repeated response loop detected: same response "
+                                            f"received {repeated_response_count + 1} times in a row. "
+                                            f"Response: {_stripped[:200]}"
+                                        )
+                                        logger.error(f"[{context_name}] {stream_error}")
+                                        safe_print(
+                                            f"[{context_name}] ERROR: {stream_error}"
+                                        )
+                                        break
+                                else:
+                                    last_response_text = _stripped
+                                    repeated_response_count = 0
                             # Check for tool concurrency error pattern in text output
                             if _is_tool_concurrency_error(block.text):
                                 detected_concurrency_error = True

apps/frontend/src/main/__tests__/rate-limit-detector.test.ts

@@ -607,6 +607,20 @@ Please authenticate and try again.`;
 
       expect(result.isAuthFailure).toBe(true);
     });
+
+    it('should NOT false-positive on AI discussion text mentioning auth topics', async () => {
+      const { detectAuthFailure } = await import('../rate-limit-detector');
+
+      // This simulates an AI PR review that discusses authentication — it should NOT trigger auth detection
+      const aiReviewText = `The PR adds authentication error detection to prevent infinite retry loops. ` +
+        `When the API returns a message like "does not have access to Claude", the system now detects it. ` +
+        `However, this pattern could also match if a user discusses authentication in a PR review. ` +
+        `We should ensure the detection is specific enough to avoid false positives. ` +
+        `Please login again is another phrase that could appear in normal discussion.`;
+      const result = detectAuthFailure(aiReviewText);
+
+      expect(result.isAuthFailure).toBe(false);
+    });
   });
 });
 

tests/test_error_utils.py

@@ -202,3 +202,106 @@ class TestIsAuthenticationError:
     def test_case_insensitive(self):
         err = Exception("UNAUTHORIZED access denied")
         assert is_authentication_error(err) is True
+
+    def test_does_not_have_access_to_claude(self):
+        """Detect 'does not have access to Claude' - returned as AI text response."""
+        err = Exception(
+            "Your account does not have access to Claude. "
+            "Please login again or contact your administrator."
+        )
+        assert is_authentication_error(err) is True
+
+    def test_please_login_again(self):
+        err = Exception("Please login again to continue.")
+        assert is_authentication_error(err) is True
+
+
+# =============================================================================
+# _is_auth_error_response (from sdk_utils)
+# =============================================================================
+
+
+class TestIsAuthErrorResponse:
+    """Tests for _is_auth_error_response() length guard in sdk_utils.
+
+    Uses importlib to load the module directly to avoid heavy package imports.
+    """
+
+    @staticmethod
+    def _load_fn():
+        """Load _is_auth_error_response without triggering runners.github.__init__."""
+        import importlib.util
+        import os
+
+        spec = importlib.util.spec_from_file_location(
+            "sdk_utils",
+            os.path.join(
+                os.path.dirname(__file__),
+                "..",
+                "apps",
+                "backend",
+                "runners",
+                "github",
+                "services",
+                "sdk_utils.py",
+            ),
+        )
+        mod = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(mod)
+        return mod._is_auth_error_response
+
+    def test_short_auth_error_detected(self):
+        """Short auth error text should be detected."""
+        fn = self._load_fn()
+        assert fn("Your account does not have access to Claude.") is True
+
+    def test_short_please_login_again(self):
+        """Short 'please login again' text should be detected."""
+        fn = self._load_fn()
+        assert fn("Please login again to continue.") is True
+
+    def test_long_ai_discussion_not_detected(self):
+        """Long AI discussion text mentioning auth phrases should NOT be detected."""
+        fn = self._load_fn()
+        long_review = (
+            "This PR adds authentication error detection to prevent infinite retry loops. "
+            "When the API returns a message like 'does not have access to Claude', the system "
+            "now detects it and stops retrying. However, this pattern could also match if a "
+            "user discusses authentication in a PR review. We should ensure the detection is "
+            "specific enough to avoid false positives. The phrase 'please login again' could "
+            "appear in normal discussion about auth flows without indicating an actual error."
+        )
+        assert len(long_review) > 300
+        assert fn(long_review) is False
+
+    def test_empty_text_not_detected(self):
+        """Empty text should not be detected."""
+        fn = self._load_fn()
+        assert fn("") is False
+
+    def test_unrelated_short_text_not_detected(self):
+        """Short text without auth phrases should not be detected."""
+        fn = self._load_fn()
+        assert fn("Task completed successfully.") is False
+
+    def test_generic_access_denied_not_detected(self):
+        """Generic 'account does not have access' should NOT trigger (too broad)."""
+        fn = self._load_fn()
+        assert fn("This account does not have access to the repository.") is False
+        assert fn("The service account does not have access to deploy.") is False
+
+    def test_boundary_exactly_300_chars_detected(self):
+        """Text of exactly 300 chars with auth phrase should be detected."""
+        fn = self._load_fn()
+        base = "does not have access to claude"  # 30 chars
+        text_300 = base + "x" * (300 - len(base))
+        assert len(text_300) == 300
+        assert fn(text_300) is True
+
+    def test_boundary_301_chars_not_detected(self):
+        """Text of 301 chars with auth phrase should NOT be detected (> 300)."""
+        fn = self._load_fn()
+        base = "does not have access to claude"  # 30 chars
+        text_301 = base + "x" * (301 - len(base))
+        assert len(text_301) == 301
+        assert fn(text_301) is False