Skip to content

RHIDP-14069: add mcp tools to unified config#2142

Open
Jdubrick wants to merge 4 commits into
lightspeed-core:mainfrom
Jdubrick:add-mcp-synthesizer
Open

RHIDP-14069: add mcp tools to unified config#2142
Jdubrick wants to merge 4 commits into
lightspeed-core:mainfrom
Jdubrick:add-mcp-synthesizer

Conversation

@Jdubrick

@Jdubrick Jdubrick commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Description

  • Adds the model-context-protocol tool group when using unified config with a profile or the default baseline run.yaml
  • Does not include it for the empty baseline -> need to use native_override
  • Adds model-context-protocol to the default baseline

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: Grok 4.5
  • Generated by: Grok 4.5

Related Tickets & Documents

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • New Features

    • Unified configuration now includes MCP support by default through the tool_runtime provider.
    • Existing tool-runtime providers, including RAG, are preserved.
    • MCP is automatically added for standard and profile-based configurations when missing.
    • Custom overrides can replace or opt out of MCP configuration.
  • Documentation

    • Updated configuration, deployment, and testing guidance to explain MCP provider behavior, including the baseline: empty exception.

Jdubrick added 4 commits July 14, 2026 12:36
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e898860b-95dc-461d-8a97-81ffbda5edc2

📥 Commits

Reviewing files that changed from the base of the PR and between 216426e and ee21a73.

📒 Files selected for processing (7)
  • docs/config.md
  • docs/deployment_guide.md
  • src/constants.py
  • src/data/default_run.yaml
  • src/llama_stack_configuration.py
  • tests/e2e/configuration/README.md
  • tests/unit/test_llama_stack_synthesize.py
📜 Recent review details
⏰ Context from checks skipped due to timeout. (16)
  • GitHub Check: list_outdated_dependencies
  • GitHub Check: build-pr
  • GitHub Check: ruff
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: Pylinter
  • GitHub Check: integration_tests (3.12)
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 1
🧰 Additional context used
📓 Path-based instructions (4)
**/*

📄 CodeRabbit inference engine (Custom checks)

**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.

Files:

  • tests/e2e/configuration/README.md
  • src/constants.py
  • docs/deployment_guide.md
  • docs/config.md
  • src/data/default_run.yaml
  • src/llama_stack_configuration.py
  • tests/unit/test_llama_stack_synthesize.py
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use absolute imports for internal Python modules and check pyproject.toml for existing dependencies and versions before adding dependencies.
All Python modules must begin with descriptive docstrings; package __init__.py files must contain brief package descriptions.
Use logger = get_logger(__name__) from log.py for module logging.
Use Final[type] for constants and check constants.py for shared constants before defining new ones.
Define type aliases at module level for clarity.
All functions must have Google-style docstrings and complete parameter and return type annotations.
Use typing_extensions.Self for model validators, modern union syntax such as str | int, and Optional[Type] for optional values.
Use descriptive, action-oriented snake_case names for functions, such as get_, validate_, and check_.
Do not modify parameter objects in place when producing results; return a new data structure instead.
Use async def for I/O operations and external API calls.
Handle APIConnectionError from Llama Stack integrations.
Use standard logger levels appropriately: debug for diagnostics, info for normal execution, warning for unexpected or potentially problematic conditions, and error for serious failures.
All classes must have descriptive Google-style docstrings, complete attribute type annotations, and specific types instead of Any.
Use PascalCase for classes with descriptive suffixes, including Configuration, Error/Exception, Resolver, and Interface where applicable.
Use ABC and @abstractmethod for abstract interfaces.
Follow Google Python docstring conventions and include Parameters, Returns, Raises, and Attributes sections when applicable.

Files:

  • src/constants.py
  • src/llama_stack_configuration.py
  • tests/unit/test_llama_stack_synthesize.py
**/*.{py,toml,yaml,yml,json}

📄 CodeRabbit inference engine (AGENTS.md)

Run and satisfy the project's configured quality checks, including Black, Pylint, Pyright, Ruff, pydocstyle, type checks, and Bandit security scanning.

Files:

  • src/constants.py
  • src/data/default_run.yaml
  • src/llama_stack_configuration.py
  • tests/unit/test_llama_stack_synthesize.py
tests/unit/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use pytest for all unit tests; do not use unittest. Put shared fixtures in conftest.py, use pytest-mock for mocks, mark async tests with pytest.mark.asyncio, and maintain at least 60% unit-test coverage.

Files:

  • tests/unit/test_llama_stack_synthesize.py
🧠 Learnings (3)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.

Applied to files:

  • src/constants.py
  • src/llama_stack_configuration.py
  • tests/unit/test_llama_stack_synthesize.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.

Applied to files:

  • src/constants.py
  • src/llama_stack_configuration.py
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.

Applied to files:

  • src/data/default_run.yaml
🪛 ast-grep (0.44.1)
src/llama_stack_configuration.py

[warning] 877-877: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(profile_path, "r", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(open-filename-from-request)

🔇 Additional comments (7)
src/constants.py (1)

163-166: LGTM!

src/data/default_run.yaml (1)

12-14: LGTM!

Also applies to: 59-61

src/llama_stack_configuration.py (1)

785-824: LGTM!

Also applies to: 856-858, 874-882, 907-916

docs/config.md (1)

830-836: LGTM!

docs/deployment_guide.md (1)

108-112: LGTM!

tests/e2e/configuration/README.md (1)

28-31: LGTM!

tests/unit/test_llama_stack_synthesize.py (1)

12-12: LGTM!

Also applies to: 21-28, 34-100, 118-125, 432-525, 527-560


Walkthrough

Unified configuration synthesis now ensures the MCP tool_runtime provider for default and profile baselines, skips that step for baseline: empty, and preserves native override behavior. Constants, defaults, documentation, and unit tests were updated accordingly.

Changes

MCP synthesis

Layer / File(s) Summary
MCP provider contract
src/constants.py, src/data/default_run.yaml
Adds MCP provider constants and includes the MCP provider in the default tool_runtime baseline.
Conditional MCP synthesis
src/llama_stack_configuration.py, docs/config.md, docs/deployment_guide.md, tests/e2e/configuration/README.md
Ensures MCP providers for non-empty baselines before applying native_override, while documenting the empty-baseline exception.
Synthesis behavior validation
tests/unit/test_llama_stack_synthesize.py
Tests MCP preservation, idempotency, default and empty baselines, overrides, and profile inputs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: max-svistunov, tisnik

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding MCP tools to the unified configuration.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed No blocking perf regressions: ensure_mcp_tool_runtime is a bounded linear scan, and synthesize_configuration adds only one extra pass; no N+1, pagination, or unbounded growth.
Security And Secret Handling ✅ Passed No new secrets/authz/injection issues: the MCP change only adds config constants/YAML entries plus an idempotent helper, with no plaintext secrets or endpoint changes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Jdubrick

Copy link
Copy Markdown
Contributor Author

cc @tisnik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant