-
Notifications
You must be signed in to change notification settings - Fork 24
Oracle MCP Server Integration #143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Oracle MCP Server Integration #143
Conversation
| @echo "Uninstalling $(AI_VIRTUAL_AGENT_RELEASE) helm chart from namespace $(NAMESPACE)" | ||
| @echo "Deleting MCPServer instances (managed by Toolhive operator)..." | ||
| @echo "Note: This will automatically clean up StatefulSets, deployments, services, and PVCs" | ||
| @oc delete mcpservers --all -n $(NAMESPACE) --ignore-not-found=true ||: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need these? These resources are installed by the helm chart, so they should be uninstalled by helm uninstall
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are installed by toolhive operator so cluster is getting into the bad state if we don't delete these before uninstalling the toolhive operator.
deploy/cluster/Makefile
Outdated
| @echo "Only run this if no other namespaces are using Toolhive." | ||
| @echo "Removing Toolhive CRDs..." | ||
| @oc delete crd mcpservers.toolhive.stacklok.dev mcptoolconfigs.toolhive.stacklok.dev mcpregistries.toolhive.stacklok.dev --ignore-not-found=true ||: | ||
| @echo "Toolhive CRDs removed." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, the crds should be uninstalled by helm I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a new change from Helm 3. Crds placed under crds directory are installed before actual installation so not included during uninstallation steps by design. These are cluster scoped so need to be removed explicitly by validating those are not used anywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we dont install toolhive automatically anymore, we listed toolhive as a prerequisite
deploy/cluster/helm/Chart.yaml
Outdated
| - name: mcp-servers | ||
| version: 0.1.0 | ||
| repository: https://rh-ai-quickstart.github.io/ai-architecture-charts | ||
| repository: "file://../../../../ai-architecture-charts/mcp-servers/helm" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # Ensure Toolhive CRDs/operator are enabled and Oracle SQLcl MCP is enabled in subchart | ||
| cmd_args+=("--set" "mcp-servers.toolhive.crds.enabled=true") | ||
| cmd_args+=("--set" "mcp-servers.toolhive.operator.enabled=true") | ||
| # Disable weather by default and enable oracle-sqlcl per new values structure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets enable toolhive by default (we can use the same flag for both crds/operator charts), additionally, lets deploy the weather mcp server by default. This means, that we will only deploy oracle23ai+oracle-sqlcl charts, and enable oracle-sqlcl mcp server
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lokeshrangineni when I think about it more, mcp-servers is just a wrapper around toolhive, so we dont even need to have a flag for installing the crds/operator charts. Whenever we install mcp-servers, we install all relevant toolhive components, and in addition we need to enable the specific mcp servers that we would like to deploy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have incorporated that logic only. I will make sure updated code is tested after incorporating the code review comments.
2d03332 to
0fac853
Compare
|
@lokeshrangineni Could you please complete the PR description form (summary, testing, screenshots, etc.) so it’s easier to review and track changes? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be safer to keep the header omitted when missing instead of sending an empty string, I would assume some services treat an empty "x-forwarded-email" as a real value rather than "not provided" in validate.py.
- Set Oracle disabled by default in values.yaml - Fix install script to use correct mcp-servers.oracle.enabled path - Enhance uninstall target to be Oracle-aware with ORACLE flag - Remove confusing step numbers from uninstall process - Add Oracle-specific cleanup when ORACLE=true - Update help documentation with Oracle examples - Clean up commented Oracle pre-install logic
- Merge Oracle MCP tool resolution from routes/virtual_assistants.py into app/api/v1/virtual_agents.py - Preserve all Oracle MCP features: tool resolution, sampling strategies, model optimizations - Update API endpoints to use LlamaStack integration with dual storage strategy - Fix backend structure to match upstream app/api/v1/ organization - Remove old routes/ directory structure - Maintain compatibility with upstream main branch while preserving Oracle MCP capabilities # Conflicts: # backend/app/api/v1/virtual_agents.py
- Revert backend/app/api/v1/virtual_agents.py to match upstream main - Remove Oracle MCP integration from this file - Keep other Oracle MCP changes intact in the branch
… variable. - Remove localDevMode from Helm values.yaml - Remove LOCAL_DEV_ENV_MODE environment variable from deployment - Remove OAuth proxy conditional logic - Remove LOCAL_DEV_MODE logic from install script - Simplify deployment to always use OAuth proxy
- Upgrade from Llama-3.2-3B-Instruct to Llama-3.1-8B-Instruct for better MCP tool support - Increase context length from 14,336 to 131,072 tokens to handle large database responses - Update Oracle SQLcl MCP server image to version 4.0.3 with verbose logging - Fix LlamaStack access policy to allow tool_group creation (create, update, delete permissions) - Increase max_infer_iters to 30 to allow complex tool-calling scenarios - Update Chart.lock with new dependencies
- Update chart versions to reflect major improvements: * llm-service: 0.1.0 → 0.2.0 (8B model support, increased context length) * llama-stack: 0.2.22 → 0.3.0 (max_infer_iters configuration) * mcp-servers: 0.1.0 → 0.2.0 (Oracle MCP server updates, permissions fix) - Update oracle23ai repository from local file path to GitHub Pages URL - All dependencies now reference published GitHub Pages repository
- Remove duplicated mcp-servers configuration from main chart values.yaml - Update Oracle SQLcl image to quay.io/lrangine/sqlcl-mcp-server:4.0.3 - Update AI Virtual Agent image repository to quay.io/lrangine/ai-virtual-agent - Consolidate MCP server configuration in ai-architecture-charts subchart - Add setup_env.sh script for environment variable management
…ing instead of empty string
edb4061 to
3047f89
Compare
- Updated Chart.yaml dependencies to use 0.5.x versions - Updated main chart version to 0.3.0 - Updated Chart.lock with helm dependency update - Updated values.yaml image tag to 0.3.0 - Updated requirements.txt llama-stack versions to 0.5.0 - Updated compose.yaml llamastack image to 0.5.0
…d during rebase - Restore skip_kb_validation parameter in create_virtual_agent_internal function - Restore parameter documentation in docstring - Restore conditional logic: if va.knowledge_base_ids and not skip_kb_validation - This parameter is used to skip KB validation when KBs are newly created and ingestion is pending
…acle related changes.
- Fixed token handling in llamastack.py (get_client parameter) - Fixed email header handling in validate.py (conditional inclusion) - Updated Docker image tag to 1.0.27-amd64 for authentication fixes - Resolved Oracle connection issues by unlocking SYSTEM account
…tion - Increased frontend request timeout from 2 to 3 minutes for large dataset queries - Updated Docker image tag to 1.0.29-amd64 in deployment values - Improved error handling for network timeouts and large responses - Incorporated code review feedback for better user experience
- Display actual error messages instead of generic interpretations - Keep timeout error guidance for user-friendly experience - Provide better debugging visibility for system errors - Simplify error handling logic for maintainability
- Remove timeout logic (AbortController and 3-minute timeout) from useChat.ts - Revert enhanced error handling to match upstream main version - Keep pagination and other improvements intact - Error handling now matches upstream main behavior
Pull Request
Description
🚀 Oracle MCP Server Integration
What's New
quay.io/lrangine/ai-virtual-agent)Key Files Changed
backend/app/api/v1/validate.py- Fixed authentication validationbackend/app/api/llamastack.py- Updated client authenticationdeploy/cluster/helm/values.yaml- Removed duplicated MCP configsdeploy/cluster/scripts/install_with_env.sh- Updated Oracle MCP configurationfrontend/src/components/chat.tsx- UI improvementsConfiguration Changes
quay.io/lrangine/ai-virtual-agent:1.0.20-amd64Breaking Changes
Ready for Review ✅
Related Issue
Fixes #
Type of Change
Components Affected
Testing
How Has This Been Tested?
Screenshots/Videos (if applicable)
Additional Notes
Is there any additional info that the reviewers should know?