Fix Issue #166: Dynamic Native LLM Provider Unavailable During Model …#167
Merged
Conversation
…Profile Construction
Root cause: ConfiguredModelProfileRegistry called BuildRegistrations() in its constructor during DI container resolution — well before LoadPluginCompositionAsync registers dynamic native providers. Custom providers were never found, causing startup to fail with Configured provider '<id>' is not available.
Changes
1. ConfiguredModelProfileRegistry.cs
Added _config backing field to store configuration for later use
Removed BuildRegistrations(config) call from the constructor (too early — dynamic providers not yet registered)
Changed DefaultProfileId from { get; } to { get; private set; } so it can be set outside the constructor
Added SetDefaultProfileId() method that defers BuildRegistrations(_config) to the correct point in startup
2. RuntimeInitializationExtensions.cs
Added services.ModelProfiles.SetDefaultProfileId() call after LoadPluginCompositionAsync (which registers dynamic native providers) and after MarkDefault() — ensuring all providers are available when profiles are built
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe runtime now defers model profile registration until ChangesDeferred model profile initialization
Documentation comparison pages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/OpenClaw.Gateway/Models/ConfiguredModelProfileRegistry.cs`:
- Around line 50-53: Repeated calls to SetDefaultProfileId() currently rebuild
registrations and overwrite owned IChatClient instances in _registrations
without disposing them, causing leaks. Update
ConfiguredModelProfileRegistry.SetDefaultProfileId to either short-circuit when
registrations are already built or dispose any previously owned clients before
calling BuildRegistrations, and make sure the cleanup covers registrations
created with ownsClient = true.
- Around line 48-53: Fallback ConfiguredModelProfileRegistry instances created
in the AdminEndpoints and DiagnosticsEndpoints fallback branches are never
initialized, leaving DefaultProfileId null and ListStatuses() empty. Update the
fallback construction path where
app.Services.GetService<IModelProfileRegistry>() and
runtime.Operations.ModelProfiles both miss by creating the
ConfiguredModelProfileRegistry and immediately calling SetDefaultProfileId()
before it is used, matching the initialization done in
RuntimeInitializationExtensions and other compatibility paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca007ddb-bebb-4afa-947d-61cbfc9d26cc
📒 Files selected for processing (7)
src/OpenClaw.Gateway/Composition/RuntimeInitializationExtensions.cssrc/OpenClaw.Gateway/GatewayLlmExecutionService.cssrc/OpenClaw.Gateway/Models/ConfiguredModelProfileRegistry.cssrc/OpenClaw.Tests/ConfiguredModelProfileRegistryTests.cssrc/OpenClaw.Tests/GatewayAdminEndpointTests.cssrc/OpenClaw.Tests/ModelProfileSelectionTests.cssrc/OpenClaw.Tests/PromptCachingTests.cs
… client leaks - Collapse suffix ternary in RuntimeInitializationExtensions.cs to stay within the GatewayStructureTests 252-line budget (Issue #166 follow-up). - Add null-guard to SetDefaultProfileId() so repeated calls short-circuit instead of overwriting _registrations and leaking owned IChatClient instances left undisposed from prior BuildRegistrations runs. - Introduce CreateInitialized() factory and use it in all three endpoint fallback paths (AdminEndpoints ×2, DiagnosticsEndpoints ×1) so fallback registries are initialized with SetDefaultProfileId() rather than left with a null DefaultProfileId and empty ListStatuses().
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/OpenClaw.Gateway/Endpoints/AdminEndpoints.cs (1)
87-95: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueReuse the fallback registry here.
CreateInitialized(startup.Config)is invoked twice on the fallback path, which can create two separate registries with owned chat clients that are never disposed. Initialize once and pass the same instance to both locals, or dispose the fallback with the app lifetime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Gateway/Endpoints/AdminEndpoints.cs` around lines 87 - 95, The fallback registry is being created twice in AdminEndpoints when resolving modelProfiles and modelEvaluationRunner, which can duplicate owned chat clients and leak resources. Update the initialization flow around the IModelProfileRegistry and ModelEvaluationRunner setup to create the ConfiguredModelProfileRegistry once on the fallback path, reuse that same instance for both locals, and ensure its lifetime is tied to the app if it is not provided by the container.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/OpenClaw.Gateway/Endpoints/AdminEndpoints.cs`:
- Around line 87-95: The fallback registry is being created twice in
AdminEndpoints when resolving modelProfiles and modelEvaluationRunner, which can
duplicate owned chat clients and leak resources. Update the initialization flow
around the IModelProfileRegistry and ModelEvaluationRunner setup to create the
ConfiguredModelProfileRegistry once on the fallback path, reuse that same
instance for both locals, and ensure its lifetime is tied to the app if it is
not provided by the container.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b33759ff-d229-4a1c-8bbc-12939e86d4c2
📒 Files selected for processing (4)
src/OpenClaw.Gateway/Composition/RuntimeInitializationExtensions.cssrc/OpenClaw.Gateway/Endpoints/AdminEndpoints.cssrc/OpenClaw.Gateway/Endpoints/DiagnosticsEndpoints.cssrc/OpenClaw.Gateway/Models/ConfiguredModelProfileRegistry.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/OpenClaw.Gateway/Models/ConfiguredModelProfileRegistry.cs
…ModelProfileRegistry as the second fallback in the modelEvaluationRunner resolution chain
// Before: two independent CreateInitialized calls could each build owned clients
?? ConfiguredModelProfileRegistry.CreateInitialized(startup.Config)
// After: reuses modelProfiles if it's a ConfiguredModelProfileRegistry
?? modelProfiles as ConfiguredModelProfileRegistry
?? ConfiguredModelProfileRegistry.CreateInitialized(startup.Config)
Now when both app.Services and runtime.Operations.ModelProfiles miss, only one fallback registry is created (for modelProfiles), and the same instance is reused for modelEvaluationRunner. The third ?? CreateInitialized is kept as a safety net for the edge case where DI provides a non-ConfiguredModelProfileRegistry implementation.
Add a comprehensive comparison between the Mode-Step Grid pattern (from cnblogs / AI老六) and OpenClaw's native MetaSKILL DAG orchestration engine — covering state modeling, execution model, failure handling, directory structure, suitability, and a universal design checklist. Available in both English and Simplified Chinese. SITE_MAP entries added for both languages.
…-CN) Add a systematic comparison between Claude Code's Dynamic Workflows (JavaScript-based executable orchestration scripts) and OpenClaw's MetaSKILL (YAML-declared DAG orchestration engine) — covering orchestration model, expressiveness, security, auditability, human- in-the-loop support, and suitability. Available in English and Simplified Chinese with SITE_MAP entries.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/SITE_MAP.md (1)
88-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSidebar Grouping omits new comparison pages.
The "Sidebar Grouping" section does not include the newly added comparison pages (
MetaSkill vs Mode-Step Grid Comparison,MetaSKILL vs Claude Code Workflows, and their Chinese counterparts). Verify whether this omission is intentional — if the sidebar grouping is meant to be a curated subset, this is fine; if it should stay in sync with the full navigation table, add the missing entries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/SITE_MAP.md` around lines 88 - 156, The Sidebar Grouping in SITE_MAP is missing the newly added comparison pages, so first confirm whether this section is meant to be a curated subset or a mirror of the full navigation. If it should stay in sync, add the missing comparison entries to the sidebar grouping alongside the existing page names, including the English and Chinese variants, and keep the list aligned with the corresponding navigation symbols in the same document.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/meta-skill-vs-claude-code-workflows.md`:
- Line 153: Update the phrasing in the docs where the compound modifier is used
so it reads with proper hyphenation, replacing “7 step kinds” with “7-step step
kinds” or rephrasing it to “7 kinds of steps”; make this wording change in the
relevant documentation section identified by the “Learning YAML structure and 7
step kinds” text.
---
Nitpick comments:
In `@docs/SITE_MAP.md`:
- Around line 88-156: The Sidebar Grouping in SITE_MAP is missing the newly
added comparison pages, so first confirm whether this section is meant to be a
curated subset or a mirror of the full navigation. If it should stay in sync,
add the missing comparison entries to the sidebar grouping alongside the
existing page names, including the English and Chinese variants, and keep the
list aligned with the corresponding navigation symbols in the same document.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d27dee7-7ee4-4a48-82bf-d8d8273d1d37
📒 Files selected for processing (6)
docs/SITE_MAP.mddocs/meta-skill-mode-step-grid-comparison.mddocs/meta-skill-vs-claude-code-workflows.mddocs/zh-CN/SITE_MAP.mddocs/zh-CN/meta-skill-mode-step-grid-comparison.mddocs/zh-CN/meta-skill-vs-claude-code-workflows.md
✅ Files skipped from review due to trivial changes (1)
- docs/zh-CN/SITE_MAP.md
make the _config field immutable by adding the readonly modifier to its declaration
Remove unrelated comparison documentation from the issue #166 fix so the pull request only carries the runtime startup change and its tests. Co-authored-by: geffzhang <geffzhang@qq.com>
Telli
approved these changes
Jun 27, 2026
Telli
left a comment
Telli
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the focused runtime change after removing unrelated docs scope. Verified the deferred profile initialization path, fallback initialization, idempotency guard, and tests. Local Release test suite passed (2,265 tests), and GitHub checks are green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
[Provide a brief description of the changes in this PR. What problem do they solve?]
Summary
Root cause: ConfiguredModelProfileRegistry called BuildRegistrations() in its constructor during DI container resolution — well before LoadPluginCompositionAsync registers dynamic native providers. Custom providers were never found, causing startup to fail with Configured provider '' is not available.
Changes
Added _config backing field to store configuration for later use Removed BuildRegistrations(config) call from the constructor (too early — dynamic providers not yet registered) Changed DefaultProfileId from { get; } to { get; private set; } so it can be set outside the constructor Added SetDefaultProfileId() method that defers BuildRegistrations(_config) to the correct point in startup
2. RuntimeInitializationExtensions.cs
Added services.ModelProfiles.SetDefaultProfileId() call after LoadPluginCompositionAsync (which registers dynamic native providers) and after MarkDefault() — ensuring all providers are available when profiles are built
Related Issues
Fixes #166
Type of Change
Validation
dotnet restore OpenClaw.Net.slnxdotnet build OpenClaw.Net.slnx --configuration Release --no-restoredotnet test OpenClaw.Net.slnx --configuration Release --no-builddotnet run --project samples/OpenClaw.HelloAgent -c Release --no-buildReview Notes
Checklist
dotnet test)Summary by CodeRabbit
Bug Fixes
Tests
Documentation