Fixed rest of GH actions tests

Author: pulk17

mod_api/middleware/auth.py

@@ -19,7 +19,6 @@
 from mod_api.middleware.error_handler import make_error_response
 from mod_api.models.api_token import ApiToken
 
-# Reused across every 401 response to keep the message consistent.
 _AUTH_FAILED_MSG = 'Bearer token is missing, expired, or invalid.'
 
 # These endpoints bypass auth entirely.
@@ -31,7 +30,8 @@
 
 def _unauthorized():
     """Shorthand for a 401 response with the standard auth failure message."""
-    return make_error_response('unauthorized', _AUTH_FAILED_MSG, http_status=401)
+    return make_error_response(
+        'unauthorized', _AUTH_FAILED_MSG, http_status=401)
 
 
 @mod_api.before_request
@@ -78,7 +78,7 @@ def authenticate_request():
 
 
 def require_scope(scope: str):
-    """Decorator: reject the request if the token lacks ``scope``."""
+    """Reject the request if the token lacks ``scope``."""
     def decorator(f):
         @functools.wraps(f)
         def decorated_function(*args, **kwargs):
@@ -88,7 +88,7 @@ def decorated_function(*args, **kwargs):
             if not token.has_scope(scope):
                 return make_error_response(
                     'forbidden',
-                    'Token does not have the required scope for this operation.',
+                    'Token lacks the required scope for this operation.',
                     details={
                         'required_scope': scope,
                         'token_scopes': token.scopes,
@@ -101,7 +101,7 @@ def decorated_function(*args, **kwargs):
 
 
 def require_roles(roles: List[str]):
-    """Decorator: reject the request if the user's role is not in ``roles``."""
+    """Reject the request if the user's role is not in ``roles``."""
     def decorator(f):
         @functools.wraps(f)
         def decorated_function(*args, **kwargs):

mod_api/middleware/error_handler.py

@@ -14,8 +14,6 @@
 
 from mod_api import mod_api
 
-# All error handlers check this prefix before intercepting — non-API
-# routes (the legacy web UI) should still get their HTML error pages.
 _API_PREFIX = '/api/v1'
 
 
@@ -86,7 +84,7 @@ def handle_404(error):
 
 @mod_api.app_errorhandler(405)
 def handle_405(error):
-    """Method not allowed."""
+    """Handle method-not-allowed errors for API routes."""
     if not _is_api_request():
         raise error
     return make_error_response(
@@ -103,7 +101,10 @@ def handle_422(error):
         raise error
     return make_error_response(
         'unprocessable',
-        getattr(error, 'description', 'Request is valid JSON but semantically invalid.'),
+        getattr(
+            error,
+            'description',
+            'Request is valid JSON but semantically invalid.'),
         http_status=422,
     )
 
@@ -123,7 +124,7 @@ def handle_429(error):
 
 @mod_api.app_errorhandler(500)
 def handle_500(error):
-    """Internal server error."""
+    """Handle unexpected server errors for API routes."""
     if not _is_api_request():
         raise error
     return make_error_response(

mod_api/middleware/validation.py

@@ -25,13 +25,12 @@
 # Whitelist of allowed sort params. Never pass raw user input to the ORM.
 ALLOWED_RUN_SORTS = frozenset([
     'created_at', '-created_at',
-    'started_at', '-started_at',
     'run_id', '-run_id',
 ])
 
 
 def validate_body(schema_class):
-    """Validate the JSON body with a Marshmallow schema, pass result as ``validated_data``."""
+    """Validate the JSON body with a schema, pass result as ``validated_data``."""
     def decorator(f):
         @wraps(f)
         def decorated(*args, **kwargs):
@@ -66,7 +65,7 @@ def decorated(*args, **kwargs):
 
 
 def validate_pagination(f):
-    """Extract and validate ``limit`` (1-100) and ``offset`` (>= 0) query params."""
+    """Extract and validate ``limit`` and ``offset`` query params."""
     @wraps(f)
     def decorated(*args, **kwargs):
         try:
@@ -75,7 +74,9 @@ def decorated(*args, **kwargs):
             return make_error_response(
                 'validation_error',
                 'limit must be an integer.',
-                details={'fields': {'limit': 'Must be an integer between 1 and 100.'}},
+                details={
+                    'fields': {
+                        'limit': 'Must be an integer between 1 and 100.'}},
                 http_status=400,
             )
 
@@ -85,7 +86,9 @@ def decorated(*args, **kwargs):
             return make_error_response(
                 'validation_error',
                 'offset must be a non-negative integer.',
-                details={'fields': {'offset': 'Must be a non-negative integer.'}},
+                details={
+                    'fields': {
+                        'offset': 'Must be a non-negative integer.'}},
                 http_status=400,
             )
 
@@ -123,14 +126,20 @@ def decorated(*args, **kwargs):
                 return make_error_response(
                     'validation_error',
                     f'{param_name} must be a positive integer.',
-                    details={'fields': {param_name: 'Must be a positive integer.'}},
+                    details={
+                        'fields': {
+                            param_name: 'Must be a positive integer.'}},
                     http_status=400,
                 )
             if int_value < 1:
                 return make_error_response(
                     'validation_error',
                     f'{param_name} must be >= 1.',
-                    details={'fields': {param_name: 'Must be >= 1. Zero and negative IDs are rejected.'}},
+                    details={
+                        'fields': {
+                            param_name: 'Must be >= 1. Zero and negative IDs are rejected.'
+                        }
+                    },
                     http_status=400,
                 )
             kwargs[param_name] = int_value
@@ -140,7 +149,7 @@ def decorated(*args, **kwargs):
 
 
 def validate_date_range(f):
-    """Parse ``created_after``/``created_before`` query params and reject inverted ranges."""
+    """Parse date query params and reject inverted ranges."""
     @wraps(f)
     def decorated(*args, **kwargs):
         from datetime import datetime
@@ -152,23 +161,29 @@ def decorated(*args, **kwargs):
 
         if created_after_str:
             try:
-                created_after = datetime.fromisoformat(created_after_str.replace('Z', '+00:00'))
+                created_after = datetime.fromisoformat(
+                    created_after_str.replace('Z', '+00:00'))
             except ValueError:
                 return make_error_response(
                     'validation_error',
                     'created_after must be a valid ISO 8601 datetime.',
-                    details={'fields': {'created_after': 'Invalid ISO 8601 format.'}},
+                    details={
+                        'fields': {
+                            'created_after': 'Invalid ISO 8601 format.'}},
                     http_status=400,
                 )
 
         if created_before_str:
             try:
-                created_before = datetime.fromisoformat(created_before_str.replace('Z', '+00:00'))
+                created_before = datetime.fromisoformat(
+                    created_before_str.replace('Z', '+00:00'))
             except ValueError:
                 return make_error_response(
                     'validation_error',
                     'created_before must be a valid ISO 8601 datetime.',
-                    details={'fields': {'created_before': 'Invalid ISO 8601 format.'}},
+                    details={
+                        'fields': {
+                            'created_before': 'Invalid ISO 8601 format.'}},
                     http_status=400,
                 )
 
@@ -202,7 +217,10 @@ def decorated(*args, **kwargs):
                 return make_error_response(
                     'validation_error',
                     f'sort must be one of: {", ".join(sorted(allowed))}',
-                    details={'fields': {'sort': f'Must be one of: {sorted(allowed)}'}},
+                    details={
+                        'fields': {
+                            'sort': f'Must be one of: {
+                                sorted(allowed)}'}},
                     http_status=400,
                 )
             kwargs['sort'] = sort

mod_api/models/api_token.py

@@ -77,35 +77,42 @@ def __init__(
         self.expires_at = self.created_at + timedelta(days=expires_in_days)
 
     def __repr__(self) -> str:
-        return f'<ApiToken {self.id} user={self.user_id} name={self.token_name}>'
+        """Return a debug representation of the token."""
+        return f'<ApiToken {self.id} user={self.user_id}>'
 
     @property
     def scopes(self) -> List[str]:
+        """Parse the JSON scopes column into a list."""
         return json.loads(self.scopes_json)
 
     @property
     def is_expired(self) -> bool:
+        """Check whether this token has passed its expiration time."""
         now = datetime.now(timezone.utc)
         expires = self.expires_at
         if expires is None:
             return True
         # MySQL DATETIME columns don't preserve tzinfo; treat naive as UTC.
         if expires.tzinfo is None:
             expires = expires.replace(tzinfo=timezone.utc)
-        return now > expires
+        return bool(now > expires)
 
     @property
     def is_revoked(self) -> bool:
-        return self.revoked_at is not None
+        """Check whether this token has been explicitly revoked."""
+        return bool(self.revoked_at is not None)
 
     @property
     def is_valid(self) -> bool:
+        """Return True if the token is neither expired nor revoked."""
         return not self.is_expired and not self.is_revoked
 
     def has_scope(self, scope: str) -> bool:
+        """Return True if the token grants the given scope."""
         return scope in self.scopes
 
     def revoke(self) -> None:
+        """Mark this token as revoked with the current timestamp."""
         self.revoked_at = datetime.now(timezone.utc)
 
     @staticmethod

mod_api/schemas/auth.py

@@ -34,6 +34,8 @@ class TokenCreateRequestSchema(Schema):
     )
 
     class Meta:
+        """Reject unknown fields."""
+
         unknown = RAISE
 
 
@@ -48,7 +50,7 @@ class AuthTokenSchema(Schema):
 
 
 class ApiTokenItemSchema(Schema):
-    """Token metadata for list responses — never includes the plaintext value."""
+    """Token metadata for list responses — never includes the plaintext."""
 
     id = fields.Integer(required=True)
     user_id = fields.Integer(required=True)
@@ -61,4 +63,5 @@ class ApiTokenItemSchema(Schema):
     revoked_at = fields.DateTime(allow_none=True)
 
     def get_scopes(self, obj):
+        """Deserialize scopes from the model's JSON column."""
         return obj.scopes

mod_api/schemas/common.py

@@ -5,13 +5,15 @@
 
 class ErrorResponseSchema(Schema):
     """Standard JSON error body returned by all error responses."""
+
     code = fields.String(required=True)
     message = fields.String(required=True)
     details = fields.Dict(keys=fields.String(), required=True, load_default={})
 
 
 class PaginationSchema(Schema):
     """Offset-based pagination metadata."""
+
     limit = fields.Integer(required=True)
     offset = fields.Integer(required=True)
     total = fields.Integer(required=True)
@@ -20,5 +22,6 @@ class PaginationSchema(Schema):
 
 class CursorPaginationSchema(Schema):
     """Cursor-based pagination metadata."""
+
     limit = fields.Integer(required=True)
     next_cursor = fields.String(allow_none=True, load_default=None)

mod_api/schemas/errors.py

@@ -5,6 +5,7 @@
 
 class ErrorItemSchema(Schema):
     """A single error derived from run results or infra progress."""
+
     error_id = fields.String(required=True)
     run_id = fields.Integer(required=True)
     sample_id = fields.Integer(allow_none=True)
@@ -22,6 +23,7 @@ class ErrorItemSchema(Schema):
 
 class ErrorSummaryBucketSchema(Schema):
     """One bucket in a grouped error summary."""
+
     key = fields.String(required=True)
     count = fields.Integer(required=True)
     severity = fields.String(required=True)
@@ -32,14 +34,17 @@ class ErrorSummaryBucketSchema(Schema):
 
 class LogLineSchema(Schema):
     """A single parsed line from a build log."""
+
     timestamp = fields.DateTime(allow_none=True)
     level = fields.String(
         required=True,
-        validate=validate.OneOf(['debug', 'info', 'warning', 'error', 'critical']),
+        validate=validate.OneOf(
+            ['debug', 'info', 'warning', 'error', 'critical']),
     )
     source = fields.String(
         required=True,
-        validate=validate.OneOf(['orchestrator', 'worker', 'build', 'test_runner', 'web']),
+        validate=validate.OneOf(
+            ['orchestrator', 'worker', 'build', 'test_runner', 'web']),
     )
     message = fields.String(required=True)
     run_id = fields.Integer(required=True)