fix: review findings + CI pipeline for MCP server tests

Review fixes:
- config.py: PORT parsing wrapped in try/except, falls back to 8080
- server.py: pinned mcp<2.0.0, documented _mcp_server private API access
- requirements.txt: mcp>=1.25.0,<2.0.0 upper bound
- Dockerfile: added HEALTHCHECK with curl to /health endpoint
- 004_api_keys.sql: CHECK constraint on tier column,
  immutable column trigger (blocks key_hash/tier/name changes for
  non-service-role), documented NULL user_id = system key behavior

CI pipeline:
- Added mcp-server/** path filter to change detection
- New test-mcp job: Python 3.13, runs pytest on mcp-server/tests/
- Security scan now also triggers on MCP changes

Author: DevanshuNEU

.github/workflows/ci.yml

@@ -17,6 +17,7 @@ jobs:
     outputs:
       backend: ${{ steps.filter.outputs.backend }}
       frontend: ${{ steps.filter.outputs.frontend }}
+      mcp: ${{ steps.filter.outputs.mcp }}
     steps:
     - uses: actions/checkout@v4
     - uses: dorny/paths-filter@v3
@@ -28,6 +29,8 @@ jobs:
             - 'railway.json'
           frontend:
             - 'frontend/**'
+          mcp:
+            - 'mcp-server/**'
 
   test-backend:
     name: Backend Tests
@@ -104,10 +107,39 @@ jobs:
       working-directory: ./frontend
       run: bun run test
 
+  test-mcp:
+    name: MCP Server Tests
+    needs: changes
+    if: ${{ needs.changes.outputs.mcp == 'true' }}
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v4
+
+    - name: Set up Python
+      uses: actions/setup-python@v5
+      with:
+        python-version: '3.13'
+        cache: 'pip'
+
+    - name: Install dependencies
+      working-directory: ./mcp-server
+      run: |
+        python -m pip install --upgrade pip
+        pip install -r requirements.txt
+
+    - name: Run tests
+      working-directory: ./mcp-server
+      env:
+        API_KEY: "test-key"
+        BACKEND_API_URL: "http://localhost:8000"
+      run: |
+        pytest tests/ -v
+
   security-scan:
     name: Security Scan
     needs: changes
-    if: ${{ needs.changes.outputs.backend == 'true' || needs.changes.outputs.frontend == 'true' }}
+    if: ${{ needs.changes.outputs.backend == 'true' || needs.changes.outputs.frontend == 'true' || needs.changes.outputs.mcp == 'true' }}
     runs-on: ubuntu-latest
 
     steps:

mcp-server/Dockerfile

@@ -14,4 +14,9 @@ ENV MCP_HOST=0.0.0.0
 
 EXPOSE 8080
 
+RUN apt-get update && apt-get install -y --no-install-recommends curl && rm -rf /var/lib/apt/lists/*
+
+HEALTHCHECK --interval=30s --timeout=5s --retries=3 \
+    CMD curl -f http://localhost:${PORT:-8080}/health || exit 1
+
 CMD ["python", "server.py"]

mcp-server/config.py

@@ -18,4 +18,8 @@
 # Transport: "stdio" for local, "streamable-http" for remote deployment
 TRANSPORT = os.getenv("MCP_TRANSPORT", "stdio")
 HOST = os.getenv("MCP_HOST", "0.0.0.0")
-PORT = int(os.getenv("PORT", "8080"))
+_port_raw = os.getenv("PORT", "8080")
+try:
+    PORT = int(_port_raw)
+except ValueError:
+    PORT = 8080

mcp-server/requirements.txt

@@ -1,5 +1,5 @@
 # MCP Server Dependencies
-mcp>=1.25.0
+mcp>=1.25.0,<2.0.0
 httpx>=0.27.0
 python-dotenv>=1.0.0
 pydantic>=2.0.0

mcp-server/server.py

@@ -37,7 +37,10 @@
 # Register tools at the low-level MCP server layer.
 # FastMCP's @tool decorator infers schemas from function signatures,
 # but we have well-tested schemas in tools.py and dispatch in handlers.py.
-_server = mcp._mcp_server
+# We access the private _mcp_server to register custom inputSchema directly.
+# TODO: monitor mcp-python for a public API to register tools with custom schemas
+#       (see: https://github.com/modelcontextprotocol/python-sdk/issues)
+_server = mcp._mcp_server  # pinned to mcp>=1.25.0,<2.0.0
 
 
 @_server.list_tools()

supabase/migrations/004_api_keys.sql

@@ -7,10 +7,13 @@
 
 CREATE TABLE IF NOT EXISTS api_keys (
   id uuid DEFAULT gen_random_uuid() PRIMARY KEY,
+  -- NULL user_id = system/service key (e.g. dogfood-mcp).
+  -- RLS policies using auth.uid() = user_id will NOT match these rows;
+  -- only the service_role (backend) can access system keys.
   user_id uuid REFERENCES auth.users(id) ON DELETE CASCADE,
   name text NOT NULL,
   key_hash text NOT NULL UNIQUE,
-  tier text DEFAULT 'free',
+  tier text DEFAULT 'free' CHECK (tier IN ('free', 'pro', 'enterprise')),
   active boolean DEFAULT true,
   created_at timestamptz DEFAULT now(),
   last_used_at timestamptz
@@ -20,10 +23,36 @@ CREATE TABLE IF NOT EXISTS api_keys (
 CREATE INDEX IF NOT EXISTS idx_api_keys_hash_active
   ON api_keys (key_hash) WHERE active = true;
 
+-- Prevent users from modifying sensitive columns via UPDATE.
+-- Only active and last_used_at can change; key_hash, tier, name are immutable.
+-- Service role (backend/admin) bypasses this check.
+CREATE OR REPLACE FUNCTION protect_api_key_immutable_cols()
+RETURNS TRIGGER AS $fn$
+BEGIN
+  IF current_setting('role', true) = 'service_role' THEN
+    RETURN NEW;
+  END IF;
+  IF NEW.key_hash IS DISTINCT FROM OLD.key_hash THEN
+    RAISE EXCEPTION 'Cannot modify key_hash';
+  END IF;
+  IF NEW.tier IS DISTINCT FROM OLD.tier THEN
+    RAISE EXCEPTION 'Cannot modify tier';
+  END IF;
+  IF NEW.name IS DISTINCT FROM OLD.name THEN
+    RAISE EXCEPTION 'Cannot modify name';
+  END IF;
+  RETURN NEW;
+END;
+$fn$ LANGUAGE plpgsql;
+
+CREATE TRIGGER api_keys_immutable_guard
+  BEFORE UPDATE ON api_keys
+  FOR EACH ROW
+  EXECUTE FUNCTION protect_api_key_immutable_cols();
+
 -- Enable RLS
 ALTER TABLE api_keys ENABLE ROW LEVEL SECURITY;
 
--- Users can only see their own keys
 CREATE POLICY "Users can view own keys"
   ON api_keys FOR SELECT
   USING (auth.uid() = user_id);