feat(models): add approved provider policy store - #14324
Merged
Merged
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe change adds canonical provider-ID resolution, cached synchronous and asynchronous policy evaluation, a versioned singleton allowlist, superuser administration endpoints, startup hydration, and cross-worker refresh handling. ChangesModel provider policy
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 |
This was referenced Jul 29, 2026
github-actions
Bot
added
enhancement
Contributor
|
✅ Migration Validation Passed All migrations follow the Expand-Contract pattern correctly. |
erichare
force-pushed
the
feat/le-1953-approved-provider-policy
branch
from
July 29, 2026 23:37
b458aee to
2dbc928
Compare
github-actions
Bot
added
enhancement
erichare
force-pushed
the
feat/le-1953-approved-provider-policy
branch
from
July 29, 2026 23:46
2dbc928 to
e16f76d
Compare
github-actions
Bot
added
enhancement
Base automatically changed from
feat/model-provider-governance
to
release-1.12.0
July 31, 2026 14:57
github-actions
Bot
added
enhancement
erichare
marked this pull request as ready for review
July 31, 2026 19:25
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
* feat(models): enforce provider policy on reads * feat(models): fail closed on denied provider runtime
github-actions
Bot
added
enhancement
* feat(models): enforce provider policy on reads * feat(models): fail closed on denied provider runtime * feat(models): govern provider configuration paths
github-actions
Bot
added
enhancement
Contributor
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/base/langflow/agentic/mcp/server.py (1)
63-75: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFix the
_services_initializedordering bug and add a regression test for it._ensure_services()sets_services_initialized = Trueonly aftermodel_provider_policy_refresh_worker.start()succeeds, so a failure in.start()leaves the guard flagFalseand permits a second, unsafe call toinitialize_services(). The existing test does not cover this path.
src/backend/base/langflow/agentic/mcp/server.py#L63-L75: move_services_initialized = Trueto immediately afterawait initialize_services()succeeds, before starting the refresh worker.src/backend/tests/unit/agentic/mcp/test_server_user_binding.py#L34-L58: after the fix lands, add a test that makesmodel_provider_policy_refresh_worker.start()raise and asserts_services_initializedis stillTrueandinitialize_servicesis not called a second time on a subsequent_ensure_services()call.🤖 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/backend/base/langflow/agentic/mcp/server.py` around lines 63 - 75, Move the _services_initialized assignment in _ensure_services immediately after initialize_services succeeds and before model_provider_policy_refresh_worker.start(), so worker-start failures leave the guard set. In src/backend/tests/unit/agentic/mcp/test_server_user_binding.py lines 34-58, add a regression test that makes the worker start raise, verifies _services_initialized remains True, and confirms a subsequent _ensure_services call does not invoke initialize_services again.
🧹 Nitpick comments (3)
src/lfx/tests/unit/services/model_provider_policy/test_policy.py (1)
199-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePatch the module reference instead of the shared
timemodule.
monkeypatch.setattr(policy_base.time, "monotonic", ...)replaces the attribute on the globaltimemodule object, becausepolicy_base.timeis that module. Every caller in the process sees a frozen clock until teardown. Bind a stub to thepolicy_base.timename instead, so the effect stays inside the module under test. The TTL boundary assertions are correct and cover both sides of the>=comparison.♻️ Proposed change to scope the clock stub
now = [100.0] - monkeypatch.setattr(policy_base.time, "monotonic", lambda: now[0]) + monkeypatch.setattr(policy_base, "time", SimpleNamespace(monotonic=lambda: now[0]))Add the import at the top of the file:
from types import SimpleNamespace🤖 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/lfx/tests/unit/services/model_provider_policy/test_policy.py` around lines 199 - 205, Update test_snapshot_cache_expires_after_ttl to replace policy_base.time with a module-local clock stub rather than mutating the shared time module; use SimpleNamespace with monotonic bound to the existing now value, and add the required types.SimpleNamespace import. Preserve the existing TTL boundary assertions.src/backend/base/langflow/services/task/model_provider_policy_refresh.py (1)
18-18: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider a longer default refresh interval.
The default poll interval is 1 second per worker process, forever. Every worker opens a new session and queries the singleton row once per second, even though the policy row changes only when an administrator submits a change. Increase the default interval (for example, to several seconds), or make it configurable, to reduce steady-state database load. The API route already pushes committed state locally at write time (
apply_model_provider_policy_state), so the polling worker only needs to catch up other workers, not deliver low-latency updates on its own.Also applies to: 55-61
🤖 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/backend/base/langflow/services/task/model_provider_policy_refresh.py` at line 18, Increase DEFAULT_MODEL_PROVIDER_POLICY_REFRESH_INTERVAL_SECONDS from its one-second default to a several-second interval to reduce steady-state database polling; preserve the existing refresh worker behavior and configurability if the surrounding implementation already supports it.src/backend/tests/unit/services/test_model_provider_policy_store.py (1)
71-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the missing-singleton-row failure path.
replace_model_provider_policy_stateraisesRuntimeErrorwhenresult.rowcount != 1(seesrc/backend/base/langflow/services/model_provider_policy.py, lines 72-74). No test in this file exercises that branch. Add a test that callsreplace_model_provider_policy_stateagainst a session/table with no singleton row and asserts theRuntimeErroris raised.As per path instructions, "verify the tests actually cover the new or changed behavior rather than acting as placeholders."
🤖 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/backend/tests/unit/services/test_model_provider_policy_store.py` around lines 71 - 80, Add a focused test in the policy store test module for replace_model_provider_policy_state that uses a session/database setup where the singleton model-provider policy row is absent, then assert the call raises RuntimeError. Ensure the test executes the real update path and verifies the missing-row failure rather than mocking or only checking SQL generation.Source: Path instructions
🤖 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.
Outside diff comments:
In `@src/backend/base/langflow/agentic/mcp/server.py`:
- Around line 63-75: Move the _services_initialized assignment in
_ensure_services immediately after initialize_services succeeds and before
model_provider_policy_refresh_worker.start(), so worker-start failures leave the
guard set. In src/backend/tests/unit/agentic/mcp/test_server_user_binding.py
lines 34-58, add a regression test that makes the worker start raise, verifies
_services_initialized remains True, and confirms a subsequent _ensure_services
call does not invoke initialize_services again.
---
Nitpick comments:
In `@src/backend/base/langflow/services/task/model_provider_policy_refresh.py`:
- Line 18: Increase DEFAULT_MODEL_PROVIDER_POLICY_REFRESH_INTERVAL_SECONDS from
its one-second default to a several-second interval to reduce steady-state
database polling; preserve the existing refresh worker behavior and
configurability if the surrounding implementation already supports it.
In `@src/backend/tests/unit/services/test_model_provider_policy_store.py`:
- Around line 71-80: Add a focused test in the policy store test module for
replace_model_provider_policy_state that uses a session/database setup where the
singleton model-provider policy row is absent, then assert the call raises
RuntimeError. Ensure the test executes the real update path and verifies the
missing-row failure rather than mocking or only checking SQL generation.
In `@src/lfx/tests/unit/services/model_provider_policy/test_policy.py`:
- Around line 199-205: Update test_snapshot_cache_expires_after_ttl to replace
policy_base.time with a module-local clock stub rather than mutating the shared
time module; use SimpleNamespace with monotonic bound to the existing now value,
and add the required types.SimpleNamespace import. Preserve the existing TTL
boundary assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a4fa4c5-e557-4e05-9cde-a383b4518a63
📒 Files selected for processing (30)
BUNDLE_API.mdsrc/backend/base/langflow/agentic/mcp/server.pysrc/backend/base/langflow/alembic/versions/e8f1a2b3c4d5_add_model_provider_policy.pysrc/backend/base/langflow/api/router.pysrc/backend/base/langflow/api/v1/__init__.pysrc/backend/base/langflow/api/v1/model_provider_policy.pysrc/backend/base/langflow/main.pysrc/backend/base/langflow/services/database/models/__init__.pysrc/backend/base/langflow/services/database/models/model_provider_policy/__init__.pysrc/backend/base/langflow/services/database/models/model_provider_policy/model.pysrc/backend/base/langflow/services/model_provider_policy.pysrc/backend/base/langflow/services/task/model_provider_policy_refresh.pysrc/backend/base/langflow/services/utils.pysrc/backend/tests/unit/agentic/mcp/test_server_user_binding.pysrc/backend/tests/unit/alembic/test_model_provider_policy_migration.pysrc/backend/tests/unit/api/v1/test_model_provider_policy.pysrc/backend/tests/unit/api/v1/test_models_provider_policy.pysrc/backend/tests/unit/services/test_model_provider_policy_store.pysrc/lfx/PLUGGABLE_SERVICES.mdsrc/lfx/src/lfx/base/models/provider_registry.pysrc/lfx/src/lfx/base/models/unified_models/instantiation.pysrc/lfx/src/lfx/base/models/unified_models/provider_queries.pysrc/lfx/src/lfx/services/model_provider_policy/__init__.pysrc/lfx/src/lfx/services/model_provider_policy/base.pysrc/lfx/src/lfx/services/model_provider_policy/service.pysrc/lfx/src/lfx/services/model_provider_policy/utils.pysrc/lfx/tests/unit/base/models/test_build_config_sticky_default.pysrc/lfx/tests/unit/base/models/test_get_llm_error_messages.pysrc/lfx/tests/unit/base/models/test_provider_registry.pysrc/lfx/tests/unit/services/model_provider_policy/test_policy.py
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #14324 +/- ##
==================================================
- Coverage 62.18% 55.04% -7.15%
==================================================
Files 2343 2328 -15
Lines 236524 230978 -5546
Branches 35151 18185 -16966
==================================================
- Hits 147088 127132 -19956
- Misses 87616 102004 +14388
- Partials 1820 1842 +22
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
Bot
added
enhancement
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
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
1 participant
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.
Summary
Scope
This branch now includes LE-1953 through LE-1956; follow-up PRs #14325, #14326, and #14327 were merged into it.
Test plan
uv run ruff check .uv run ruff format --check .