chore: add CI/CD pipeline and update documentation#7
Merged
Conversation
Add SAP HANA MCP server to README.md and CLAUDE.md with complete package description, architecture details, and repository structure updates. Also document mamba-mcp-fs utility modules (content.py, errors.py).
- Created internal/docs/codebase-analysis.md with comprehensive analysis of all 4 packages, including architecture overview, critical files, patterns, challenges, and recommendations - Updated CLAUDE.md with Key Patterns to Follow (7 core patterns for server packages), Testing Conventions, and Known Inconsistencies to guide future development - Updated README.md with Architecture section explaining the shared FastMCP + layered tool pattern and tech stack
…mba_mcp_hana Resolve module name asymmetry where mamba-mcp-hana was the only package that didn't follow the 1:1 naming convention (it used mamba_mcp_sap_hana with an extra sap_ prefix). All other packages map directly: mamba-mcp-pg → mamba_mcp_pg, mamba-mcp-fs → mamba_mcp_fs. Changes: - Rename src/mamba_mcp_sap_hana/ → src/mamba_mcp_hana/ - Update pyproject.toml entry point and build config - Update all imports across source and test files - Update CLAUDE.md, README.md, and codebase-analysis.md BREAKING CHANGE: Module import path changed from mamba_mcp_sap_hana to mamba_mcp_hana
Create mamba-mcp-core to extract duplicated code from server packages: - cli.py: validate_env_file, resolve_default_env_file, setup_logging - config.py: module-level env file path state management - errors.py: flat ToolError model + create_tool_error factory with server-specific suggestions_map parameter - fuzzy.py: Levenshtein distance + find_similar_names with scaled threshold (max(2, min(len//2, 5))) — standardizes PG vs HANA approach - transport.py: normalize_transport accepting both "http" and "streamable-http" Includes 51 tests covering all modules. Addresses Challenges #3 (code duplication) and #6 (fuzzy matching inconsistency).
Replace duplicated code across all three server packages with shared imports from mamba-mcp-core. This eliminates ~500 lines of copy-pasted CLI utilities, config state management, error models, and fuzzy matching. Changes per server: - PG: core imports for CLI, config, errors, fuzzy; dict return preserved - FS: core imports for CLI, config, transport; exception hierarchy kept - HANA: core imports for CLI, config, errors, fuzzy, transport; removed redundant resolve_env_file(); ToolError return preserved; suggest_similar wrapper maps max_suggestions→max_results Transport pattern updated to accept both "http" and "streamable-http" in all servers, with normalize_transport() applied before mcp.run(). Test updates: - HANA: removed TestLevenshteinDistance (now in core), removed TestResolveEnvFile (now in core), removed _levenshtein_distance import - All existing tests pass (PG: 66, HANA: 778, Core: 51)
…S pattern Change all 11 HANA tool functions from returning `str` (JSON) to returning `OutputModel | dict[str, Any]` to match the PG/FS convention. Tool changes (4 files, 11 tools): - Return type annotations: `-> str` → `-> SpecificOutput | dict[str, Any]` - Success paths: `result.model_dump_json()` → `result` (return model directly) - Error paths: `error.model_dump_json()` → `error.model_dump()` (return dict) - Added output model imports to each tool file Test changes (4 files): - Removed `json.loads(result)` calls throughout - Success assertions: dict key access → model attribute access - Error assertions: `json.loads(result)["code"]` → `result["code"]` - Added `isinstance` checks for result types - Removed unused `import json` statements All 778 HANA tests pass.
…s, server, and models Add 205 new tests across 4 test files to match HANA/FS test coverage: - test_config.py: DatabaseSettings, ServerSettings, nested settings, env file state - test_errors.py: ErrorCode constants, ERROR_SUGGESTIONS completeness, create_tool_error wrapper - test_server.py: FastMCP instance, AppContext dataclass, app_lifespan lifecycle - test_models.py: All Pydantic models with validation constraints, serialization, edge cases
- Add GitHub Actions CI workflow with lint, type-check, and per-package test matrix (core, pg, fs, hana) - Update CLAUDE.md: fix test commands for per-package execution, update Known Inconsistencies to reflect resolved items - Update codebase-analysis.md: mark all 8 challenges as resolved
There was a problem hiding this comment.
Pull request overview
This PR adds a CI/CD pipeline via GitHub Actions and introduces a new mamba-mcp-core shared utilities package to eliminate code duplication across the three MCP server packages (PostgreSQL, Filesystem, SAP HANA). The PR also renames the HANA package from mamba_mcp_sap_hana to mamba_mcp_hana for consistency, and updates documentation to reflect all resolved challenges.
Changes:
- Added GitHub Actions CI workflow with lint, type-check, and per-package test jobs
- Created
mamba-mcp-corepackage with shared utilities (CLI helpers, config state, error models, fuzzy matching, transport normalization) - Refactored PG, FS, and HANA packages to use core utilities instead of duplicated code
- Renamed HANA package module from
mamba_mcp_sap_hanatomamba_mcp_hana - Updated documentation (CLAUDE.md, README.md, codebase-analysis.md) to mark challenges as resolved
Reviewed changes
Copilot reviewed 66 out of 73 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/ci.yml | New CI pipeline with 3 jobs (lint, type-check, test matrix) |
| packages/mamba-mcp-core/* | New shared utilities package with cli, config, errors, fuzzy, transport modules |
| packages/mamba-mcp-pg/src/mamba_mcp_pg/* | Refactored to import core utilities (config, cli, errors, transport) |
| packages/mamba-mcp-hana/src/mamba_mcp_hana/* | Renamed from mamba_mcp_sap_hana + imported core utilities |
| packages/mamba-mcp-fs/src/mamba_mcp_fs/* | Refactored to import core utilities (config, cli, transport) |
| uv.lock, pyproject.toml | Added mamba-mcp-core to workspace and dependencies |
| CLAUDE.md, README.md | Updated with core package info and resolved challenges |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+14
to
+16
| - uses: astral-sh/setup-uv@v4 | ||
| with: | ||
| enable-cache: true |
There was a problem hiding this comment.
The CI workflow uses actions/checkout@v4 and astral-sh/setup-uv@v4, which are valid as of the knowledge cutoff (January 2025). However, the workflow should specify a Python version explicitly to ensure consistency across CI runs and prevent potential issues if the default Python version changes in the GitHub runners.
| - mamba-mcp-core | ||
| - mamba-mcp-pg | ||
| - mamba-mcp-fs | ||
| - mamba-mcp-hana |
There was a problem hiding this comment.
The test matrix excludes mamba-mcp-client, but according to the PR description and documentation, the client package exists and should also have its tests run in CI. This omission means client tests won't be executed automatically, potentially allowing bugs to slip through.
Suggested change
| - mamba-mcp-hana | |
| - mamba-mcp-hana | |
| - mamba-mcp-client |
| async with app_lifespan(mock_server): | ||
| raise RuntimeError("something broke") | ||
|
|
||
| mock_dispose.assert_called_once_with(mock_engine) |
There was a problem hiding this comment.
This statement is unreachable.
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
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
.github/workflows/ci.yml) with 3 job types: lint, type-check, and per-package test matrixCI Architecture
lintruff checkandruff format --checkacross all packagestype-checkmypyacross all packagestest(matrix)pytestper-package for core, pg, fs, and hana independentlyDesign decisions:
conftest.pyimport conflictsfail-fast: falseensures all packages report independentlyenable-cache: truefor fast dependency resolutionTest plan