LCORE-1834: MCP Approval Configuration#1773
Conversation
WalkthroughThis PR adds human-in-the-loop approval configuration for MCP tool invocations. New ChangesApproval Configuration Implementation
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/openapi.json (1)
11403-11483:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegenerate the OpenAPI snapshot before merge.
CI already shows
docs/openapi.jsondiffers from the generated schema, so this checked-in contract is stale. Please re-runuv run scripts/generate_openapi_schema.py docs/openapi.jsonand commit the regenerated output instead of hand-editing these sections.Also applies to: 12065-12068, 14318-14334, 15340-15340
🤖 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/openapi.json` around lines 11403 - 11483, The checked-in OpenAPI snapshot is stale: regenerate the schema using the repository's OpenAPI generation script (generate_openapi_schema.py) and commit the updated docs/openapi.json so the "ApprovalFilter-Input", "ApprovalFilter-Output", "ApprovalsConfiguration" (and other affected schema sections) match the generated output; run the generator, review the diff, and replace the manual edits with the generated file before merging.
🤖 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 `@tests/unit/models/config/README.md`:
- Around line 9-10: Add a single blank line immediately after the heading "##
[test_approvals_configuration.py](test_approvals_configuration.py)" in the
README so there is an empty line between the heading and the following
paragraph, satisfying markdownlint MD022 (blanks-around-headings).
---
Outside diff comments:
In `@docs/openapi.json`:
- Around line 11403-11483: The checked-in OpenAPI snapshot is stale: regenerate
the schema using the repository's OpenAPI generation script
(generate_openapi_schema.py) and commit the updated docs/openapi.json so the
"ApprovalFilter-Input", "ApprovalFilter-Output", "ApprovalsConfiguration" (and
other affected schema sections) match the generated output; run the generator,
review the diff, and replace the manual edits with the generated file before
merging.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 21bf3f5c-2ac0-4c07-a974-2d921bcb3580
📒 Files selected for processing (8)
docs/config.mddocs/openapi.jsonsrc/configuration.pysrc/models/config.pytests/unit/models/config/README.mdtests/unit/models/config/test_approvals_configuration.pytests/unit/models/config/test_dump_configuration.pytests/unit/models/config/test_model_context_protocol_server.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build-pr
- GitHub Check: unit_tests (3.13)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal modules:from authentication import get_auth_dependency
Llama Stack imports: Usefrom llama_stack_client import AsyncLlamaStackClient
Checkconstants.pyfor shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Useasync deffor I/O operations and external API calls
Use standard log levels with clear purposes:debug()for diagnostic info,info()for program execution,warning()for unexpected events,error()for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes:Configuration,Error/Exception,Resolver,Interface
Abstract classes must use ABC with@abstractmethoddecorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes
Files:
src/configuration.pysrc/models/config.py
src/**/configuration.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/configuration.py: All config models must extendConfigurationBasewithextra="forbid"to reject unknown fields
Use@field_validatorand@model_validatorfor custom validation in Pydantic models
Files:
src/configuration.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/unit/models/config/test_dump_configuration.pytests/unit/models/config/test_model_context_protocol_server.pytests/unit/models/config/test_approvals_configuration.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Pydantic models must use
@model_validatorand@field_validatorfor validation and complete type annotations for all attributes, avoidingAnytype
Files:
src/models/config.py
🧠 Learnings (2)
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/config.py
🪛 GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt
docs/openapi.json
[error] 1-1: CI check failed: docs/openapi.json is out of date compared to the generated schema. Diff detected between docs/openapi.json and /tmp/openapi-generated.json. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json
🪛 GitHub Actions: OpenAPI (Spectral) / spectral
docs/openapi.json
[error] 1-1: OpenAPI schema is out of date. 'diff -u docs/openapi.json /tmp/openapi-generated.json' failed, so CI reports: docs/openapi.json is out of date. Regenerate with: 'uv run scripts/generate_openapi_schema.py docs/openapi.json'.
🪛 markdownlint-cli2 (0.22.1)
tests/unit/models/config/README.md
[warning] 9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (6)
tests/unit/models/config/test_approvals_configuration.py (1)
1-122: LGTM!tests/unit/models/config/test_model_context_protocol_server.py (1)
11-11: LGTM!Also applies to: 29-30, 32-67
tests/unit/models/config/test_dump_configuration.py (1)
8-8: LGTM!Also applies to: 29-40, 215-215, 300-300, 358-358, 364-364, 370-370, 564-564, 812-812, 1040-1040, 1258-1258
src/models/config.py (1)
470-527: LGTM!Also applies to: 617-625, 2123-2127
docs/config.md (1)
65-97: LGTM!Also applies to: 242-242, 457-457
src/configuration.py (1)
17-17: LGTM!Also applies to: 463-476
| "description": "Filter configuration for restricting which MCP tools can be used.\n\n:param tool_names: (Optional) List of specific tool names that are allowed" | ||
| }, | ||
| "ApprovalFilter": { | ||
| "ApprovalFilter-Input": { |
There was a problem hiding this comment.
This rename is caused by name conflict between our configuration model and Llama Stack's model that is part of InputToolMCP model.
jrobertboos
left a comment
jrobertboos
left a comment
There was a problem hiding this comment.
LGTM, It would maybe be nice to have an example config file that shows how to make use of the approvals tho in the examples dir.
3 participants
Description
Adds configuration blocks for MCP server approval policy.
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Documentation