Make ProviderRegistry.initialize() idempotent with partial-failure cleanup

What happened

PR #3124 wired ProviderRegistry.initialize() into the FastAPI lifespan. The review agent flagged two related issues at low severity: (1) initialize() has no cleanup on partial failure — if the Nth provider fails during await provider.initialize(), previously-initialized providers remain in _providers, leaving the singleton in a half-initialized state; (2) no tests cover multi-provider partial-failure or empty-providers configuration paths. The human reviewer approved with "LGTM" without addressing these findings, and the PR was merged at 11:55 UTC on 2026-11-03.

What could go better

The review agent correctly identified a fragile contract in ProviderRegistry.initialize(). In production, the service currently aborts on startup failure, making partial state benign for now. However, the method's contract is fragile: if callers ever retry initialization (e.g., during graceful degradation or hot-reload), the half-initialized state becomes a real bug — stale providers from a previous call persist alongside new ones. The singleton pattern amplifies this because state leaks across calls. Confidence: medium — the current production path is safe, but the contract should be hardened before the registry is used in more complex lifecycle scenarios. The missing tests are separately valuable as regression guards for the initialization logic.

Proposed change

In src/providers/inference/registry.py, modify ProviderRegistry.initialize() to: (1) clear self._providers at the top of the method so retries start from a clean slate, and (2) build the new provider mapping in a local variable and assign it to self._providers only after all providers initialize successfully, so partial failure leaves the registry empty rather than half-populated.

In tests/unit/app/test_main_startup.py, add tests for: (1) multi-provider partial failure — configure two providers where the second fails, assert the registry is empty after the error; (2) empty providers list — configure inference.providers = [], assert the registry initializes successfully with zero providers; (3) re-initialization idempotency — call initialize() twice with different configurations, assert only the second configuration's providers remain.

Validation criteria

Unit tests for partial failure, empty providers, and re-initialization idempotency pass. The next review agent run on a PR touching registry.py should not raise the edge-case or missing-test findings.

Related issues checked

Searched: ProviderRegistry initialize partial failure cleanup idempotent; inference provider registry initialize test coverage startup
No overlapping issues found.

Generated by retro agent from #3124

🔗 Related PRs

#3124 - LCORE-4710: Implement registry initialization [closed]

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

Create Plan

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

🤖 Finished Triage · ✅ Success · Started 12:18 PM UTC · Completed 12:22 PM UTC

Commit: fa901c9 · View workflow run →

Runtime: pi · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.06

Severity: low — the current production path aborts on startup failure, so the half-initialized state is never observed. The fix hardens the initialize() contract for future retry/reload scenarios.

Recommended approach: Build the provider mapping in a local dict, then assign it to self._providers atomically after all providers succeed. This follows the project's own anti-pattern guidance in AGENTS.md about avoiding in-place parameter modification.

The existing test test_initialize_adds_to_existing_providers explicitly asserts the current additive behavior and will need updating to expect replacement semantics instead.

Proposed test cases:

@pytest.mark.asyncio
async def test_partial_failure_leaves_registry_empty():
    """When the Nth provider fails, no providers remain registered."""
    # Configure two providers; make the second raise on initialize()
    # Assert registry.list_providers() == [] after the error

@pytest.mark.asyncio
async def test_empty_providers_initializes_successfully():
    """Empty providers list yields a valid registry with zero providers."""

@pytest.mark.asyncio
async def test_reinitialize_replaces_previous_providers():
    """Second initialize() replaces first configuration entirely."""

Next steps:

/fs-code — agent creates a PR to implement this issue
/fs-code <your instruction> — agent implements with your specific guidance

🤖 Finished Code · ✅ Success · Started 12:26 PM UTC · Completed 12:42 PM UTC

Commit: fa901c9 · View workflow run →

Runtime: pi · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.36

Navigation Menu

Repository navigation

Make ProviderRegistry.initialize() idempotent with partial-failure cleanup #3129

What happened

What could go better

Proposed change

Validation criteria

Related issues checked

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Make ProviderRegistry.initialize() idempotent with partial-failure cleanup #3129

Description

What happened

What could go better

Proposed change

Validation criteria

Related issues checked

Activity

coderabbitai commented on Nov 3, 2026

fullsend-ai-triage commented on Nov 3, 2026

fullsend-ai-triage commented on Nov 3, 2026

fullsend-ai-coder commented on Nov 3, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions