Compare commits

...

7 Commits

Author SHA1 Message Date
Barcode Betty 7b595744e1 fix(api): mypy no-redef and no-any-return errors on dev (CAR-1335)
CI / lint (pull_request) Successful in 5s
CI / typecheck (pull_request) Successful in 18s
CI / test (pull_request) Successful in 22s
CI / build-and-push (pull_request) Has been skipped
The api typecheck job is continue-on-error but still posts a failure
status that blocks merges. Three pre-existing mypy errors on dev were
inherited by every PR based on it:

1. middleware/rate_limit.py: duplicate 'name already defined' for
   _public_limiter, _auth_limiter, _auth_strict_limiter (declared at
   lines 111-113 and again at 124-126). The second set is redundant
   because actual assignment happens inside the if/else below.
2. cache.py:43 - 'Returning Any' from .get(); the redis client's get()
   return type isn't narrowed to bytes|str, so the final 'return value'
   branch is Any. Wrap with str() to satisfy the declared str|None.
3. middleware/rate_limit.py:150 - 'Returning Any' from _get_client_ip.
   request.headers.get() and request.client.host are typed Any; wrap
   the branches with str() to match the declared str return.

Refs CAR-1335.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-09 05:25:41 +00:00
Barcode Betty 4877513bbf style: ruff format conformance (CAR-1335)
- tests/test_openapi.py: collapse 2 blank lines to 1 (ruff format)
- tests/conftest.py: collapse 2 blank lines to 1 (ruff format)

These format nits block lint (a hard gate). The conftest.py one was
introduced in CAR-1132 (#42) and would have blocked every subsequent PR
on dev until fixed.

Refs CAR-1335, CAR-1135.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-09 05:23:36 +00:00
Barcode Betty 9e46bdc460 fix(api): document dispose_engine lazy import + regression test (CAR-1135)
CI / lint (pull_request) Failing after 4s
CI / typecheck (pull_request) Failing after 18s
CI / test (pull_request) Successful in 22s
CI / build-and-push (pull_request) Has been skipped
- main.py: add docstring inside the lifespan function explaining why
  dispose_engine is lazy-imported rather than top-level. The original
  import path (top-level) crashed the container at import time with
  'ImportError: cannot import name dispose_engine from cartsnitch_api.database'
  when database.py was stale or stripped during a CI build. Lazy import
  keeps the engine disposal behavior while preventing the module-load
  crash.
- tests/test_openapi.py: add test_dispose_engine_importable_from_database
  that asserts dispose_engine is importable and callable. This is the
  exact path the deployed UAT image was failing on, captured as a
  regression test so a future regression lands in CI before deploy.

Refs CAR-1135.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-09 05:13:08 +00:00
Savannah Savings 3860a5d061 Merge pull request 'Fix CAR-1132: SQLite UUID binding and User.id defaults in test fixtures' (#42) from betty/car-1132-comprehensive-fix into dev
CI / lint (push) Failing after 7s
CI / typecheck (pull_request) Failing after 18s
CI / test (pull_request) Successful in 22s
CI / build-and-push (pull_request) Has been skipped
CI / build-and-push (push) Has been skipped
CI / typecheck (push) Failing after 17s
CI / lint (pull_request) Failing after 3s
CI / test (push) Successful in 22s
2026-06-09 01:01:09 +00:00
Savannah Savings 7a7aaca064 Fix PostgreSQL connection pool issues (CAR-1077) (#39)
CI / build-and-push (push) Has been skipped
CI / test (pull_request) Failing after 1m2s
CI / build-and-push (pull_request) Has been skipped
CI / lint (push) Successful in 5s
CI / typecheck (push) Successful in 28s
CI / lint (pull_request) Successful in 6s
CI / test (push) Failing after 1m0s
CI / typecheck (pull_request) Successful in 29s
QA approved by Checkout Charlie; CTO Dev review approved by Savannah Savings. Adds pool_timeout=30 and DB-connectivity /health probe. Strict CI improvement (lint+typecheck green); remaining test failure pre-existing on dev, tracked under CAR-1132/PR#42.
2026-06-02 15:10:01 +00:00
Barcode Betty 76781ed238 style: fix ruff format in conftest.py
CI / lint (pull_request) Successful in 5s
CI / typecheck (pull_request) Successful in 29s
CI / test (pull_request) Failing after 1m0s
CI / build-and-push (pull_request) Has been skipped
Add missing blank line between the _set_timestamp_defaults helper
and the next top-level constant so `ruff format --check .` passes.
Pre-existing on dev's HEAD; surfaced after rebasing PR #39 onto dev
in 2b20946.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-02 14:58:18 +00:00
Barcode Betty 2b20946ad7 fix: /health returns 503 on DB failure, pool_timeout=30, CI typecheck fixes
CI / lint (pull_request) Failing after 4s
CI / typecheck (pull_request) Failing after 25s
CI / test (pull_request) Failing after 1m5s
CI / build-and-push (pull_request) Has been skipped
QA review of PR #39 (CAR-1121) identified three blocking issues; this
commit addresses all three plus the typecheck errors flagged as CI RED.

CAR-1077 (PR #39) changes:
- database.py: add pool_timeout=30 so the engine fails fast when the
  connection pool is exhausted (defends against the "server closed
  connection unexpectedly" pod failures).
- routes/health.py: /health now calls SELECT 1 through Depends(get_db)
  and raises HTTPException(503) when the database is unreachable, so
  Kubernetes readiness probes can correctly mark the pod unhealthy and
  stop routing traffic to it.  Logs the failure at exception level for
  observability.
- Drop .mcp.json from this PR (root-level MCP server config, not
  related to the pool fix; tracked separately).

CI typecheck fixes (pre-existing on dev, were failing mypy on PR #39):
- auth/passwords.py: cast bcrypt return values so mypy doesn't widen
  to Any.
- config.py: silence the false-positive call-arg on Settings() — the
  three required fields are populated from the environment by
  pydantic-settings at runtime.
- cache.py: coerce the bytes/str union returned by the redis client
  to the documented str | None return type.
- middleware/rate_limit.py: annotate the three module-level limiters
  with the RateLimitBackend protocol, cast the redis zrange score to
  float before arithmetic, and add max_requests/window_seconds to the
  protocol so the response-header builder can read them.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-02 14:53:16 +00:00
8 changed files with 53 additions and 19 deletions
-11
View File
@@ -1,11 +0,0 @@
{
"mcpServers": {
"gitea": {
"type": "http",
"url": "https://git-mcp.farh.net/mcp",
"headers": {
"Authorization": "Bearer ${GITEA_TOKEN}"
}
}
}
}
+2 -2
View File
@@ -4,8 +4,8 @@ import bcrypt
def hash_password(password: str) -> str:
return bcrypt.hashpw(password.encode(), bcrypt.gensalt()).decode()
return str(bcrypt.hashpw(password.encode(), bcrypt.gensalt()).decode())
def verify_password(plain_password: str, hashed_password: str) -> bool:
return bcrypt.checkpw(plain_password.encode(), hashed_password.encode())
return bool(bcrypt.checkpw(plain_password.encode(), hashed_password.encode()))
+1 -1
View File
@@ -40,7 +40,7 @@ class CacheClient:
return None
if isinstance(value, bytes):
return value.decode("utf-8", errors="replace")
return value
return str(value)
async def set(self, key: str, value: str, ttl_seconds: int = 300) -> None:
if not self._client:
+1
View File
@@ -14,6 +14,7 @@ def _build_engine_kwargs() -> dict:
kwargs.update(
pool_size=10,
max_overflow=20,
pool_timeout=30,
pool_pre_ping=True,
pool_recycle=3600,
)
+6
View File
@@ -25,6 +25,12 @@ from cartsnitch_api.routes.user import router as user_router
@asynccontextmanager
async def lifespan(app: FastAPI):
# Lazy import: keep `dispose_engine` out of the top-level imports so a
# stale or partially-built database.py never breaks module load on
# container start. The function is required for graceful pool cleanup
# on shutdown; if the import fails, the cache_client.close() that
# follows the yield would mask it. See CAR-1135 for the original
# ImportError that motivated this pattern.
from cartsnitch_api.database import dispose_engine
await cache_client.initialize()
+2 -2
View File
@@ -147,8 +147,8 @@ def _get_client_ip(request: Request) -> str:
"""Extract client IP, respecting X-Forwarded-For behind a reverse proxy."""
forwarded = request.headers.get("x-forwarded-for")
if forwarded:
return forwarded.split(",")[0].strip()
return request.client.host if request.client else "unknown"
return str(forwarded.split(",")[0].strip())
return str(request.client.host) if request.client else "unknown"
def _get_rate_limit_key(request: Request) -> tuple[str, RateLimitBackend]:
+27 -3
View File
@@ -1,16 +1,40 @@
"""Health check and error metrics endpoints."""
from fastapi import APIRouter, Depends
import logging
from fastapi import APIRouter, Depends, HTTPException, status
from sqlalchemy import text
from sqlalchemy.ext.asyncio import AsyncSession
from cartsnitch_api.auth.dependencies import verify_service_key
from cartsnitch_api.database import get_db
from cartsnitch_api.middleware.error_handler import get_error_monitor
logger = logging.getLogger(__name__)
router = APIRouter(tags=["health"])
@router.get("/health")
async def health():
return {"status": "ok"}
async def health(db: AsyncSession = Depends(get_db)):
"""Liveness + DB connectivity probe.
Returns HTTP 200 when the API process is responsive *and* the database
is reachable, so Kubernetes readiness probes can correctly route traffic
away from pods that have lost their database connection.
Returns HTTP 503 when the database is unreachable so K8s marks the pod
unhealthy and stops sending traffic to it.
"""
try:
await db.execute(text("SELECT 1"))
except Exception as exc:
logger.exception("Health check failed: database unreachable")
raise HTTPException(
status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
detail={"status": "unavailable", "database": "disconnected"},
) from exc
return {"status": "ok", "database": "connected"}
@router.get("/internal/error-stats", dependencies=[Depends(verify_service_key)])
+14
View File
@@ -3,8 +3,22 @@
import pytest
from httpx import ASGITransport, AsyncClient
from cartsnitch_api.database import dispose_engine
from cartsnitch_api.main import app
def test_dispose_engine_importable_from_database():
"""Regression for CAR-1135: api main.py used to import dispose_engine
at module level. A stale database.py (no dispose_engine) crashed the
container at import time with ImportError on line 9. The fix moved
the import inside the lifespan function, but `dispose_engine` must
still be importable from `cartsnitch_api.database` for the lifespan
teardown to actually close pooled connections.
"""
assert callable(dispose_engine)
assert dispose_engine.__name__ == "dispose_engine"
EXPECTED_ROUTES = [
# Auth (3 — register/login/refresh are handled by Better-Auth service)
("get", "/auth/me"),