Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion run_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import os
import random
import re
import ssl
import sys
import tempfile
import time
Expand Down Expand Up @@ -10680,7 +10681,7 @@ def _stop_spinner():
# errors (ValueError, TypeError) are programming bugs.
is_local_validation_error = (
isinstance(api_error, (ValueError, TypeError))
and not isinstance(api_error, UnicodeEncodeError)
and not isinstance(api_error, (UnicodeEncodeError, ssl.SSLError)) # ssl.SSLError subclasses ValueError via OSError — transport error, not a programming bug
)
is_client_error = (
is_local_validation_error
Expand Down
77 changes: 77 additions & 0 deletions tests/run_agent/test_14367_ssl_local_validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
"""Tests for #14367 — ssl.SSLCertVerificationError mis-classified as local validation error.

ssl.SSLCertVerificationError inherits from SSLError → OSError → ValueError.
The is_local_validation_error guard in run_agent.py caught it via isinstance(e, ValueError)
and triggered a non-retryable abort, even though the error classifier returns retryable=True
for all OSError subclasses (transport heuristic, step 7).

Fix: add ssl.SSLError to the exclusion tuple alongside UnicodeEncodeError.
"""

import ssl


# ── Helper: replicate the fixed guard from run_agent.py ───────────────────

def _is_local_validation_error(api_error: BaseException) -> bool:
"""Mirror of the is_local_validation_error check in run_agent.py."""
return (
isinstance(api_error, (ValueError, TypeError))
and not isinstance(api_error, (UnicodeEncodeError, ssl.SSLError))
)


# ── Tests ──────────────────────────────────────────────────────────────────

class TestSSLErrorNotLocalValidation:
"""ssl.SSLError and its subclasses must not be treated as local
validation errors — they are transport errors and should be retried."""

def test_ssl_cert_verification_error_is_not_local(self):
"""SSLCertVerificationError (the reported bug case) should not be
classified as a local validation error."""
e = ssl.SSLCertVerificationError("certificate verify failed: self-signed certificate")
assert not _is_local_validation_error(e)

def test_ssl_error_base_is_not_local(self):
"""ssl.SSLError itself (parent of all SSL errors) should not be
classified as a local validation error."""
e = ssl.SSLError("unknown SSL error")
assert not _is_local_validation_error(e)

def test_ssl_cert_verification_error_is_value_error_via_mro(self):
"""Confirm the MRO bug exists: SSLCertVerificationError IS a ValueError,
which is why the pre-fix guard misfired."""
e = ssl.SSLCertVerificationError("cert error")
assert isinstance(e, ValueError), (
"Prerequisite: SSLCertVerificationError must subclass ValueError via MRO"
)


class TestExistingExclusionsPreserved:
"""Existing exclusions (UnicodeEncodeError) must remain effective."""

def test_unicode_encode_error_is_not_local(self):
"""Pre-existing: UnicodeEncodeError should not be a local validation error."""
e = UnicodeEncodeError("utf-8", "x", 0, 1, "invalid char")
assert not _is_local_validation_error(e)


class TestValueErrorAndTypeErrorStillLocal:
"""Plain ValueError and TypeError remain local validation errors — the fix
must not weaken detection of real programming bugs."""

def test_plain_value_error_is_local(self):
e = ValueError("unexpected keyword argument 'stream_options'")
assert _is_local_validation_error(e)

def test_plain_type_error_is_local(self):
e = TypeError("got an unexpected keyword argument 'stream_options'")
assert _is_local_validation_error(e)

def test_json_decode_error_is_still_local(self):
"""json.JSONDecodeError is a ValueError subclass — still local until
#14366 lands its own exclusion."""
import json
e = json.JSONDecodeError("Expecting value", "doc", 0)
assert _is_local_validation_error(e)