fix(api): handle 403 subscription errors without retrying

fewtarius · fewtarius · commit 0923d5967dde · 2026-04-20T20:04:13.000-04:00
Problem:
- Ollama returns 403 with plain string errors (not hash errors)
- ResponseHandler was only checking hash errors for subscription keywords
- Even when permanent failure was detected, WorkflowOrchestrator still
  counted it against session error budget and retried
- User saw 'Maximum retries exceeded: Authentication token refreshed'
  instead of the actual subscription error

Solution:
- ResponseHandler: Check $error directly for subscription keywords
  (handles both hash and plain string error formats)
- WorkflowOrchestrator: Return immediately on error_type=auth_failed
  (bypasses session error budget and retry logic)

Testing:
- All 29 unit tests pass
- Integration test shows correct error returned on first attempt
diff --git a/lib/CLIO/Core/API/ResponseHandler.pm b/lib/CLIO/Core/API/ResponseHandler.pm
@@ -403,6 +403,18 @@ sub handle_error_response {
         };
         my $user_message = _get_rate_limit_user_message($rate_limit_info);
 
+        # Also check for Copilot-style quota messages ("You've used X% of your session rate limit")
+        # These come in error_obj.message or error_obj.reason and should be preserved as system_message
+        if (!$user_message && $error_obj && ref($error_obj) eq 'HASH') {
+            my $quota_msg = $error_obj->{message} // $error_obj->{reason} // '';
+            # Match both "you've used" and "you have used" patterns
+            if ($quota_msg =~ /you(?:'ve| have) used \d+%? of your? (session )?rate limit/i ||
+                $quota_msg =~ /percent_remaining/i) {
+                $user_message = $quota_msg;
+                log_info('ResponseHandler', "Captured Copilot quota message: $quota_msg");
+            }
+        }
+
         # Weekly/monthly limits don't reset quickly - don't use misleading retry_after header
         # The header might say "retry in 1 second" but the actual limit takes days to reset
         log_debug('ResponseHandler', "Rate limit check: detected_code=$detected_rate_limit_code, retry_after=$retry_after, reset_ts=$reset_timestamp");
@@ -640,6 +652,27 @@ sub handle_error_response {
             $retry_info = sprintf("API rate limit exceeded. Retrying in %d seconds.", $retry_after);
             $error = $retry_info;
         }
+
+        # Handle Copilot-style quota messages ("You've used X% of your session rate limit")
+        # These are non-retryable - the user needs to wait for reset, can't fix with retry
+        if ($user_message && $user_message =~ /you(?:'ve| have) used \d+%? of your? (session )?rate limit/i) {
+            $is_retryable_error = 0;
+            $retryable = 0;
+            $retry_after = 0;
+            $error_type = 'rate_limit';
+            $error = $user_message;  # Return the message directly, no "Retrying in X seconds"
+            log_info('ResponseHandler', "Copilot session rate limit detected (non-retryable): $user_message");
+
+            my $copilot_result = { success => 0, error => $error, _error => $error };
+            $copilot_result->{retryable} = 0;
+            $copilot_result->{retry_after} = 0;
+            $copilot_result->{error_type} = 'rate_limit';
+            $copilot_result->{rate_limit_code} = 'copilot_session_limit';
+            $copilot_result->{system_message} = $user_message;
+            $copilot_result->{error_obj} = $error_obj if $error_obj;
+            log_debug('ResponseHandler', "Final error being returned: $error");
+            return $copilot_result;
+        }
     }
     # Handle quota exceeded errors (non-retryable - user must take action)
     # Detected via semantic codes in error body, not HTTP status
@@ -659,25 +692,63 @@ sub handle_error_response {
         log_info('ResponseHandler', "Quota exceeded (code=$error_obj->{code}): $user_message");
     }
     # Handle authentication failures (401, 403)
+    # RFC 9110: 401 = "I don't know you" (invalid credentials), 403 = "I know you but you're not allowed"
+    # We treat these differently:
+    #   - 401: Token is invalid, recovery makes sense
+    #   - 403: Check if it's a permanent failure (subscription required, model unavailable) before recovering
     elsif ($status == 401 || $status == 403) {
-        log_info('ResponseHandler', "Authentication error ($status), attempting token recovery");
+        # For 403, check if the error message indicates a permanent failure that token recovery won't fix
+        my $is_permanent_auth_failure = 0;
+        my $original_error_msg = $error;  # Preserve original error for permanent failures
+
+        if ($status == 403) {
+            # Check for subscription/upgrade/payment required errors
+            # These are permanent - retrying won't help
+            # Handle both hash errors ({message => "...", code => "..."}) and plain string errors
+            my $err_msg = '';
+            if (ref($error_obj) eq 'HASH') {
+                $err_msg = $error_obj->{message} // '';
+            } elsif (!ref($error_obj)) {
+                # Plain string error - use the error string directly
+                $err_msg = "$error_obj";
+            }
 
-        my $recovered = 0;
-        if ($attempt_token_recovery) {
-            $recovered = $attempt_token_recovery->();
+            if ($err_msg =~ /subscription|upgrade|paid|requires? (a |the )?(subscription|model|plan)/i) {
+                $is_permanent_auth_failure = 1;
+                log_info('ResponseHandler', "403 permanent auth failure detected (subscription/upgrade required): $err_msg");
+            }
         }
 
-        if ($recovered) {
-            $is_retryable_error = 1;
-            $retryable = 1;
-            $retry_after = 1;
-            $error_type = 'auth_recovered';
-            $retry_info = "Authentication token refreshed. Retrying request...";
-            $error = $retry_info;
-        } else {
-            $error = "Authentication failed (HTTP $status). Your token may have expired or been revoked. "
-                   . "Please run /api logout then /api login to re-authenticate.";
+        if ($is_permanent_auth_failure) {
+            # Permanent failure - don't attempt recovery, just report the original error
+            $is_retryable_error = 0;
+            $retryable = 0;
             $error_type = 'auth_failed';
+            # Preserve the actual provider error message
+            $error = $original_error_msg;
+            log_info('ResponseHandler', "Returning permanent 403 error without recovery attempt");
+        }
+        else {
+            # Potentially transient auth failure (401, or 403 without subscription keywords)
+            log_info('ResponseHandler', "Authentication error ($status), attempting token recovery");
+
+            my $recovered = 0;
+            if ($attempt_token_recovery) {
+                $recovered = $attempt_token_recovery->();
+            }
+
+            if ($recovered) {
+                $is_retryable_error = 1;
+                $retryable = 1;
+                $retry_after = 1;
+                $error_type = 'auth_recovered';
+                $retry_info = "Authentication token refreshed. Retrying request...";
+                $error = $retry_info;
+            } else {
+                $error = "Authentication failed (HTTP $status). Your token may have expired or been revoked. "
+                       . "Please run /api logout then /api login to re-authenticate.";
+                $error_type = 'auth_failed';
+            }
         }
     }
     # Handle transient server errors (5xx except 599 which is handled as connection_error)
diff --git a/lib/CLIO/Core/WorkflowOrchestrator.pm b/lib/CLIO/Core/WorkflowOrchestrator.pm
@@ -2065,12 +2065,12 @@ sub _handle_api_error {
         return 'retry';
     }
 
-    # ── Non-retryable rate limit handling (weekly/monthly limits) ─────
+    # ── Non-retryable rate limit handling (weekly/monthly limits and Copilot session limits) ─────
     # These don't reset quickly - return immediately with error, don't retry
     if (defined($api_response->{error_type}) && $api_response->{error_type} eq 'rate_limit' && !$api_response->{retryable}) {
         my $rl_code = $api_response->{rate_limit_code} // '';
-        if ($rl_code =~ /user_weekly_rate_limited|user_monthly_rate_limited/i) {
-            log_info('WorkflowOrchestrator', "Weekly/monthly rate limit detected ($rl_code) - returning error without retry");
+        if ($rl_code =~ /user_weekly_rate_limited|user_monthly_rate_limited|copilot_session_limit/i) {
+            log_info('WorkflowOrchestrator', "Non-retryable rate limit detected ($rl_code) - returning error without retry");
             return {
                 success         => 0,
                 error           => $api_response->{error},
@@ -2081,6 +2081,18 @@ sub _handle_api_error {
         }
     }
 
+    # ── Non-retryable auth failures (403 subscription errors) ──────────────────────────────────
+    # These are permanent failures - token recovery won't help
+    if (defined($api_response->{error_type}) && $api_response->{error_type} eq 'auth_failed') {
+        log_info('WorkflowOrchestrator', "Permanent auth failure detected - returning error immediately");
+        return {
+            success         => 0,
+            error           => $api_response->{error},
+            iterations      => $iteration,
+            tool_calls_made => $tool_calls_made,
+        };
+    }
+
     # ── Non-retryable errors ──────────────────────────────────────────
     $$retry_count_ref = 0;
 
diff --git a/tests/unit/test_response_handler.pl b/tests/unit/test_response_handler.pl
@@ -324,4 +324,90 @@
     is($result->{error_type}, 'connection_error', 'Error type is connection_error');
 };
 
+# =============================================================================
+# 401/403 handling tests
+# =============================================================================
+
+subtest 'handle_error_response - 403 subscription required (permanent failure)' => sub {
+    my $handler = CLIO::Core::API::ResponseHandler->new();
+    my $resp = MockResponse->new(
+        code => 403,
+        status_line => '403 Forbidden',
+        content => '{"error":{"message":"this model requires a subscription, upgrade for access: https://ollama.com/upgrade","code":"subscription_required"}}',
+    );
+
+    # No recovery callback - should return immediately with the subscription error
+    my $result = $handler->handle_error_response($resp, '{}', 0);
+    is($result->{retryable}, 0, 'Subscription 403 is NOT retryable');
+    is($result->{error_type}, 'auth_failed', 'Error type is auth_failed');
+    like($result->{error}, qr/subscription|upgrade/i, 'Error contains subscription message');
+};
+
+subtest 'handle_error_response - 403 plain string error (ollama style)' => sub {
+    my $handler = CLIO::Core::API::ResponseHandler->new();
+    # Simulate Ollama's plain string error - error_obj is just a string, not a hash
+    my $resp = MockResponse->new(
+        code => 403,
+        status_line => '403 Forbidden',
+        content => '{"error":"this model requires a subscription, upgrade for access: https://ollama.com/upgrade"}',
+    );
+
+    my $result = $handler->handle_error_response($resp, '{}', 0);
+    is($result->{retryable}, 0, 'Subscription 403 is NOT retryable (plain string error)');
+    is($result->{error_type}, 'auth_failed', 'Error type is auth_failed');
+    like($result->{error}, qr/subscription|upgrade/i, 'Error contains subscription message');
+};
+
+subtest 'handle_error_response - 403 with recovery callback (transient auth issue)' => sub {
+    my $handler = CLIO::Core::API::ResponseHandler->new();
+    my $resp = MockResponse->new(
+        code => 403,
+        status_line => '403 Forbidden',
+        content => '{"error":{"message":"Token invalid","code":"token_invalid"}}',
+    );
+
+    # With recovery callback that succeeds
+    my $result = $handler->handle_error_response($resp, '{}', 0,
+        attempt_token_recovery => sub { return 1; }
+    );
+    is($result->{retryable}, 1, '403 with recovery is retryable');
+    is($result->{error_type}, 'auth_recovered', 'Error type is auth_recovered');
+    like($result->{error}, qr/refreshed/i, 'Error says token refreshed');
+};
+
+subtest 'handle_error_response - 401 with recovery (invalid token)' => sub {
+    my $handler = CLIO::Core::API::ResponseHandler->new();
+    my $resp = MockResponse->new(
+        code => 401,
+        status_line => '401 Unauthorized',
+        content => '{"error":{"message":"Invalid token"}}',
+    );
+
+    # With recovery callback that succeeds
+    my $result = $handler->handle_error_response($resp, '{}', 0,
+        attempt_token_recovery => sub { return 1; }
+    );
+    is($result->{retryable}, 1, '401 with recovery is retryable');
+    is($result->{error_type}, 'auth_recovered', 'Error type is auth_recovered');
+};
+
+# =============================================================================
+# Copilot session rate limit tests
+# =============================================================================
+
+subtest 'handle_error_response - Copilot session rate limit message' => sub {
+    my $handler = CLIO::Core::API::ResponseHandler->new();
+    my $resp = MockResponse->new(
+        code => 429,
+        status_line => '429 Too Many Requests',
+        content => '{"error":{"message":"You have used 51% of your session rate limit","code":"session_limit"}}',
+    );
+
+    my $result = $handler->handle_error_response($resp, '{}', 0);
+    is($result->{retryable}, 0, 'Copilot session limit is NOT retryable');
+    is($result->{error_type}, 'rate_limit', 'Error type is rate_limit');
+    is($result->{rate_limit_code}, 'copilot_session_limit', 'Rate limit code is copilot_session_limit');
+    like($result->{error}, qr/51%.*session.*rate.*limit/i, 'Error contains the quota message');
+};
+
 done_testing();
