🐛 Bug-fix PR

Closes #2987

📌 Summary

Fixes a bug where gateway registration failures (e.g., tool schema validation errors) leave orphaned gateway records in the database, blocking subsequent registration attempts with the same name.

The root cause is missing db.rollback() calls in exception handlers, along with two other contributing issues:

• the structured logger committing the caller's dirty session.
• the before_commit_handler silently swallowing flush exceptions.

Key changes:

• Added db.rollback() to 13 exception handlers across 4 methods in gateway_service.py
• IMPORTANT ONE: Isolated structured logger DB persistence using fresh_db_session()
• Made before_commit_handler propagate flush exceptions instead of swallowing them
• Updated all affected tests to match new behavior

🔁 Reproduction Steps

1. Register a gateway with a tool that has an invalid schema (e.g., missing required inputSchema field)
2. Registration fails with a validation error (expected)
3. Attempt to register the same gateway name again
4. Bug: Second registration fails with "Gateway name already exists" even though the first registration failed
5. The orphaned gateway record has no tools associated with it

🐞 Root Cause

Three issues combine to cause the bug:

1. Primary cause: Missing db.rollback() in exception handlers:
   In register_gateway(), after db.add(db_gateway) and db.flush() succeed (persisting the gateway row), tool validation fails during db.flush() of tool records. The except* handlers catch the error but never call db.rollback(), leaving the gateway object in the session's dirty state.

2. Contributing factor 1: before_commit_handler swallows exceptions:
   db.py's before_commit_handler wraps session.flush() in a bare except Exception: pass, silently discarding any flush failure during commit. This means even when db.commit() is eventually called, the failed flush doesn't cause the commit to fail.

3. Contributing factor 2: Structured logger commits caller's session:
   structured_logger._persist_to_database() was called with the caller's db session and called db.add(log_entry) + db.commit() on it. This inadvertently committed the orphaned gateway record along with the log entry.

The chain: db.flush() fails on tools -> exception handler skips rollback -> structured_logger.log(db=db) calls db.commit() on same session -> before_commit_handler swallows re-flush failure -> gateway record persisted without tools -> name permanently reserved.

💡 Fix Description

1. Added db.rollback() to all exception handlers (gateway_service.py)
   Added explicit db.rollback() calls before any logging or re-raising in all exception handlers across 4 methods:

   • register_gateway - 7 except* handlers (validation errors, integrity errors, connection errors, timeout errors, general exceptions)
   • update_gateway - 3 exception handlers (name conflict, not found, integrity error)
   • set_gateway_state - 1 exception handler (permission error)
   • fetch_tools_after_oauth - 2 exception handlers (connection error, general exception)
     db.rollback() is always safe to call - it's a no-op on a clean session with no pending changes.

2. Isolated structured logger persistence (structured_logger.py)
   Changed _persist_to_database() to use fresh_db_session() instead of the caller's session. This ensures:

   • Log persistence can never accidentally commit the caller's dirty state
   • Logs are persisted even when the business transaction fails
   • A broken caller session doesn't prevent log persistence
   • Complete session lifecycle isolation (fresh_db_session handles commit/rollback/close)

3. Propagate flush exceptions in before_commit_handler (db.py)
   Removed the bare except Exception: pass from before_commit_handler. The handler now simply calls session.flush() without catching exceptions, so flush failures properly cause the commit to fail, triggering rollback in get_db().

4. Updated tests

   • test_structured_logger.py - Updated 7 tests to mock fresh_db_session instead of mock_db; replaced 2 tests with fresh-session-aware equivalents
   • test_logging_service_comprehensive.py - Updated 2 tests for fresh_db_session pattern
   • test_db.py - Updated before_commit_handler test to verify exception propagation instead of swallowing

🧪 Verification

📐 MCP Compliance (if relevant)

• Matches current MCP spec
• No breaking change to MCP clients

✅ Checklist

• Code formatted (make black isort pre-commit)
• No secrets/credentials committed