50. Dependency Singleton Pattern Justification
Date: 2025-11-10Status
AcceptedCategory
Core ArchitectureContext and Problem Statement
During a comprehensive review of OpenAI Codex findings (2025-11-10), a potential concern was raised about the use of module-level singleton instances for dependency injection insrc/mcp_server_langgraph/core/dependencies.py.
The current implementation uses global variables with lazy initialization:
Decision Drivers
- FastAPI best practices: Standard dependency injection pattern
- Performance: Avoid recreating expensive clients on every request
- Test isolation: Tests need to override dependencies without global state pollution
- Simplicity: Implementation should be straightforward and maintainable
- Production stability: Pattern must work reliably in production environment
Considered Options
Option 1: Keep Current Singleton Pattern ✅ (SELECTED)
Description: Continue using module-level singletons with FastAPI dependency injection. Pros:- Standard FastAPI pattern - widely used and documented
- Performance: Clients created once, reused across requests
- FastAPI
app.dependency_overridesprovides clean test isolation - Simple implementation - easy to understand and maintain
- No global state leakage when using dependency overrides correctly
- Proven pattern in production (zero issues reported)
- Tests must use
app.dependency_overrides(if they don’t, could share state) - Runtime config changes won’t affect already-initialized singletons
- Cannot support multiple Keycloak realms simultaneously (not a requirement)
Option 2: Provider Classes
Description: Refactor to lightweight provider classes instead of module-level globals.- Encapsulates state in class instead of module
- Potentially easier to add multi-realm support later
- More “object-oriented” approach
- More boilerplate code for same functionality
- Still has singleton behavior (just wrapped in class)
- Doesn’t solve any actual problems with current implementation
- More complex without clear benefit
Option 3: FastAPI Lifespan State
Description: Use FastAPI’s lifespan context manager to manage dependencies.- Explicit lifecycle management
- Clear initialization and cleanup points
- State attached to app instance
- More complex initialization
- Clients must be accessed via
request.app.state.clients - Harder to test (need to mock app.state)
- Breaking change to existing codebase
- No clear advantage over current pattern
Decision Outcome
Chosen option: Option 1 - Keep Current Singleton PatternRationale
- Standard FastAPI Pattern: This is the recommended approach in FastAPI documentation for dependency injection with expensive resources.
- Production Proven: Zero issues reported in production with current pattern. The concern is theoretical, not practical.
-
Test Coverage Validates Safety: We have comprehensive test coverage (
tests/unit/test_dependencies_wiring.py) that validates:- Dependency overrides work correctly
- Test isolation is maintained
- No state leakage between tests
-
Codex Finding Assessment: The original concern was classified as “PARTIAL CONCERN” - valid design pattern but could cause issues in specific scenarios. Our analysis shows those scenarios don’t apply:
- ✅ Tests properly use
app.dependency_overrides - ✅ Runtime config changes not needed (config loaded at startup)
- ✅ Multi-tenancy/multi-realm not required
- ✅ Tests properly use
-
Refactoring Cost vs Benefit: Switching to another pattern would:
- Require extensive refactoring across codebase
- Introduce risk of bugs during migration
- Provide zero measurable benefit
- Not solve any actual production problems
Example: Correct Test Pattern
Consequences
Positive
- No breaking changes: Existing code continues to work
- Maintain performance: Single client instances per process
- Clear testing pattern: Documented in TESTING.md
- Proven stability: Continue using production-tested pattern
Negative
- Must educate developers: Ensure new contributors understand correct testing pattern
- Documentation critical: Must document when to use dependency overrides
Neutral
- Future flexibility preserved: If requirements change (e.g., multi-realm support), can refactor then
- Pattern remains standard: No deviation from FastAPI best practices
Implementation Guidelines
For Application Code
For Tests
For Test Fixtures (tests/conftest.py)
Monitoring and Validation
Test Coverage
- ✅
tests/unit/test_dependencies_wiring.py- Validates dependency injection - ✅ All API tests use
app.dependency_overridescorrectly - ✅ Zero test failures due to state leakage
Production Monitoring
- ✅ No memory leaks reported
- ✅ No client exhaustion issues
- ✅ Clean startup/shutdown in all environments
Pre-commit Validation
- ✅ Tests validate proper fixture usage
- ✅ Documentation kept current in TESTING.md
Related Decisions
- ADR-0042: Dependency Injection Configuration Fixes - Fixed critical bugs in dependency initialization
- ADR-0049: Pytest Fixture Consolidation - Consolidated duplicate fixtures for better test organization
References
- FastAPI Dependency Injection
- FastAPI Testing with Overrides
- OpenAI Codex Validation Report (2025-11-10): Finding classified as “PARTIAL CONCERN - valid pattern with acceptable trade-offs”
- TESTING.md “Regression Test Patterns - Pattern 4: Mock-Based Tests Hiding Bugs”
Review History
- 2025-11-10: Initial decision - Keep current pattern (validated during Codex findings review)