Skip to content

Add user-managed environment variables and Metabase MCP server support - #11

Merged
manuelkiessling merged 13 commits into
mainfrom
devin/1758455555-user-managed-env-vars-metabase
Sep 24, 2025
Merged

Add user-managed environment variables and Metabase MCP server support#11
manuelkiessling merged 13 commits into
mainfrom
devin/1758455555-user-managed-env-vars-metabase

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 21, 2025

Copy link
Copy Markdown
Contributor

Add user-managed environment variables and Metabase MCP server support

Summary

Implements a generic system for users to provide custom environment variables to MCP instances via the UI, then adds a new "Metabase" instance type that integrates the metabase-mcp-server and requires METABASE_URL and METABASE_API_KEY credentials.

Key Changes:

  • New mcp_instance_environment_variables table with proper entity relationships
  • UI for managing key-value pairs on instance detail pages
  • Environment variables passed to Docker containers during creation
  • New Metabase MCP server Docker image and configuration
  • Comprehensive unit test coverage (15 tests across 4 test classes)

Review & Testing Checklist for Human

This is a medium-risk change that touches database, container orchestration, and user input handling.

  • Test environment variable UI end-to-end - Create an instance, add/modify/delete environment variables, verify they persist and appear in container
  • Verify container environment merging - Check that both instance type defaults AND user-defined variables are passed to containers correctly
  • Test Metabase instance creation - Create a Metabase instance with required METABASE_URL and METABASE_API_KEY, verify container starts successfully
  • Security review - Environment variables are stored as plain text in database - confirm this is acceptable for the intended use case
  • Validate key format restrictions - Test that invalid environment variable names (spaces, special chars) are properly rejected

Recommended Test Plan:

  1. Run migrations on staging environment
  2. Create test Metabase instance with real credentials
  3. Add various environment variables (valid/invalid keys, special characters in values)
  4. Verify container logs show correct environment variables
  5. Test UI edge cases (empty keys, mismatched arrays, very long values)

Notes

  • Database Migration Note: Two migrations were required due to SQL reserved word issue with key column (renamed to env_key)
  • Container Integration: Fixed existing gap where instance type environment variables weren't being merged into containers
  • Testing: All 15 unit tests pass, covering entity behavior, domain logic, controller validation, and container integration

Requested by: @manuelkiessling
Link to Devin run: https://app.devin.ai/sessions/a0eaccf3cd904767b484d892cdbf9aeb

- Implement generic user-managed environment variable system using mapped table approach
- Create McpInstanceEnvironmentVariable entity with proper Doctrine relationships
- Add UI for managing environment variables in instance detail page
- Fix existing gap where instance type env vars weren't merged into container creation
- Add Metabase MCP v1 instance type with required METABASE_URL and METABASE_API_KEY support
- Create Docker image for metabase-mcp-server integration
- Use proper table naming conventions (mcp_instance_environment_variables)
- Resolve SQL reserved word issue by using env_key column name
- Follow Hexagonal Architecture patterns with proper layer separation

Co-Authored-By: manuel@kiessling.net <manuel@kiessling.net>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

devin-ai-integration Bot and others added 2 commits September 21, 2025 13:05
- Add proper mock implementations for addFlash and redirectToRoute methods
- Fix return type compatibility with Symfony's AbstractController
- Update URL assertions to match mock redirect behavior
- All 6 controller tests now pass successfully

Co-Authored-By: manuel@kiessling.net <manuel@kiessling.net>
- Fix code style issues in McpInstancesDomainServiceTest.php
- Fix code style issues in InstancesControllerEnvironmentVariablesTest.php
- Ensure consistent formatting across all test files

Co-Authored-By: manuel@kiessling.net <manuel@kiessling.net>
devin-ai-integration Bot and others added 10 commits September 21, 2025 16:58
…-side architecture

- Add comprehensive client-side architecture documentation to archbook.md
- Document Stimulus controller organization within feature boundaries
- Add client-side testing patterns to software-test-design.mdc
- Update AssetMapper to include feature-specific asset paths
- Create environment_variables_controller.js using Stimulus framework
- Refactor instances_detail.html.twig to use Twig Stimulus helpers
- Replace inline JavaScript with proper Stimulus controller
- Use semantic HTML <template> element for row templates
- Add data-test attributes for stable UI testing
- Maintain all existing functionality while improving code organization

Co-Authored-By: manuel@kiessling.net <manuel@kiessling.net>
…ocs-update-v2

Refactor environment variables UI to use Stimulus and document client-side architecture
@manuelkiessling
manuelkiessling merged commit 3ae9755 into main Sep 24, 2025
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