fix: module ID version handling inconsistency across tools (#11)

* docs: add rational on readme

* docs: add MCP inspector and remove list of tools from readme

* fix: bring consistency around definition of module_id

Author: vburckhardt

test/test_get_module_details.py

@@ -129,17 +129,25 @@ class TestGetModuleDetailsValidation:
     def test_valid_module_id_parsing(self):
         """Test that valid module IDs are parsed correctly."""
         # This will be implemented when we create the actual function
-        from tim_mcp.tools.details import parse_module_id
+        from tim_mcp.utils.module_id import parse_module_id
 
         # Test standard format
         namespace, name, provider = parse_module_id("terraform-ibm-modules/vpc/ibm")
         assert namespace == "terraform-ibm-modules"
         assert name == "vpc"
         assert provider == "ibm"
 
+        # Test with version included
+        from tim_mcp.utils.module_id import parse_module_id_with_version
+        namespace, name, provider, version = parse_module_id_with_version("terraform-ibm-modules/vpc/ibm/1.2.3")
+        assert namespace == "terraform-ibm-modules"
+        assert name == "vpc"
+        assert provider == "ibm"
+        assert version == "1.2.3"
+
     def test_invalid_module_id_format(self):
         """Test validation fails for invalid module ID format."""
-        from tim_mcp.tools.details import parse_module_id
+        from tim_mcp.utils.module_id import parse_module_id
 
         with pytest.raises(ValidationError) as exc_info:
             parse_module_id("invalid-format")
@@ -149,17 +157,19 @@ def test_invalid_module_id_format(self):
 
     def test_empty_module_id_components(self):
         """Test validation fails for empty components."""
-        from tim_mcp.tools.details import parse_module_id
+        from tim_mcp.utils.module_id import parse_module_id
 
-        with pytest.raises(ValidationError):
+        with pytest.raises(ValidationError) as exc_info:
             parse_module_id("//")
 
+        assert exc_info.value.field == "module_id"
+
     def test_module_id_with_extra_slashes(self):
-        """Test validation fails for module ID with extra slashes."""
-        from tim_mcp.tools.details import parse_module_id
+        """Test validation fails for module ID with too many slashes."""
+        from tim_mcp.utils.module_id import parse_module_id
 
         with pytest.raises(ValidationError):
-            parse_module_id("namespace/name/provider/extra")
+            parse_module_id("namespace/name/provider/version/extra")
 
 
 class TestGetModuleDetailsSuccess:
@@ -221,7 +231,7 @@ async def test_get_module_details_specific_version(
             ]
 
             request = ModuleDetailsRequest(
-                module_id="terraform-ibm-modules/vpc/ibm", version="7.4.1"
+                module_id="terraform-ibm-modules/vpc/ibm/7.4.1"
             )
 
             result = await get_module_details_impl(request, config)

test/test_list_content.py

@@ -11,7 +11,7 @@
 import pytest
 
 from tim_mcp.config import Config
-from tim_mcp.exceptions import GitHubError, ModuleNotFoundError, RateLimitError
+from tim_mcp.exceptions import GitHubError, ModuleNotFoundError, RateLimitError, ValidationError
 from tim_mcp.tools.list_content import list_content_impl
 from tim_mcp.types import ListContentRequest
 
@@ -119,7 +119,7 @@ async def test_list_content_success(
         """Test successful content listing with version latest."""
         # Setup
         request = ListContentRequest(
-            module_id="terraform-ibm-modules/vpc/ibm", version="latest"
+            module_id="terraform-ibm-modules/vpc/ibm"
         )
 
         mock_github_client.get_repository_info.return_value = sample_repo_info
@@ -189,7 +189,7 @@ async def test_list_content_with_specific_version(
         """Test content listing with specific version tag."""
         # Setup
         request = ListContentRequest(
-            module_id="terraform-ibm-modules/vpc/ibm", version="v5.1.0"
+            module_id="terraform-ibm-modules/vpc/ibm/v5.1.0"
         )
 
         mock_github_client.get_repository_info.return_value = sample_repo_info
@@ -223,7 +223,7 @@ async def test_list_content_module_not_found(self, mock_config, mock_github_clie
         """Test error handling when module repository is not found."""
         # Setup
         request = ListContentRequest(
-            module_id="nonexistent/module/ibm", version="latest"
+            module_id="nonexistent/module/ibm"
         )
 
         mock_github_client.get_repository_info.side_effect = ModuleNotFoundError(
@@ -247,7 +247,7 @@ async def test_list_content_github_api_error(self, mock_config, mock_github_clie
         """Test error handling for GitHub API errors."""
         # Setup
         request = ListContentRequest(
-            module_id="terraform-ibm-modules/vpc/ibm", version="latest"
+            module_id="terraform-ibm-modules/vpc/ibm"
         )
 
         mock_github_client.get_repository_info.side_effect = GitHubError(
@@ -273,7 +273,7 @@ async def test_list_content_rate_limit_error(self, mock_config, mock_github_clie
         """Test error handling for GitHub rate limit errors."""
         # Setup
         request = ListContentRequest(
-            module_id="terraform-ibm-modules/vpc/ibm", version="latest"
+            module_id="terraform-ibm-modules/vpc/ibm"
         )
 
         mock_github_client.get_repository_tree.side_effect = RateLimitError(
@@ -301,13 +301,13 @@ async def test_list_content_rate_limit_error(self, mock_config, mock_github_clie
     async def test_list_content_invalid_module_id(self, mock_config):
         """Test error handling for invalid module ID format."""
         # Setup
-        request = ListContentRequest(module_id="invalid-module-id", version="latest")
+        request = ListContentRequest(module_id="invalid-module-id")
 
         # Execute & Verify
-        with pytest.raises(ModuleNotFoundError) as exc_info:
+        with pytest.raises(ValidationError) as exc_info:
             await list_content_impl(request, mock_config)
 
-        assert "Invalid module ID format" in str(exc_info.value.details)
+        assert "Invalid module_id format" in str(exc_info.value)
 
     @pytest.mark.asyncio
     async def test_list_content_empty_repository(
@@ -316,7 +316,7 @@ async def test_list_content_empty_repository(
         """Test content listing for empty repository."""
         # Setup
         request = ListContentRequest(
-            module_id="terraform-ibm-modules/empty/ibm", version="latest"
+            module_id="terraform-ibm-modules/empty/ibm"
         )
 
         mock_github_client.get_repository_info.return_value = sample_repo_info
@@ -348,7 +348,7 @@ async def test_list_content_readme_parsing_fallback(
         """Test README parsing with fallback when content is not available."""
         # Setup
         request = ListContentRequest(
-            module_id="terraform-ibm-modules/vpc/ibm", version="latest"
+            module_id="terraform-ibm-modules/vpc/ibm"
         )
 
         mock_github_client.get_repository_info.return_value = sample_repo_info
@@ -406,7 +406,7 @@ async def test_list_content_path_categorization(
         ]
 
         request = ListContentRequest(
-            module_id="terraform-ibm-modules/test/ibm", version="latest"
+            module_id="terraform-ibm-modules/test/ibm"
         )
 
         mock_github_client.get_repository_info.return_value = sample_repo_info

tim_mcp/server.py

@@ -225,7 +225,7 @@ async def search_modules(
 
 
 @mcp.tool()
-async def get_module_details(module_id: str, version: str = "latest") -> str:
+async def get_module_details(module_id: str) -> str:
     """
     Get structured module metadata from Terraform Registry - for understanding module interface when writing NEW terraform.
 
@@ -247,8 +247,7 @@ async def get_module_details(module_id: str, version: str = "latest") -> str:
     - Module dependencies
 
     Args:
-        module_id: Full module identifier (e.g., "terraform-ibm-modules/vpc/ibm")
-        version: Specific version or "latest" (default: "latest")
+        module_id: Full module identifier (e.g., "terraform-ibm-modules/vpc/ibm" or "terraform-ibm-modules/vpc/ibm/1.2.3")
 
     Returns:
         Plain text with markdown formatted module details including inputs, outputs, and description
@@ -257,7 +256,7 @@ async def get_module_details(module_id: str, version: str = "latest") -> str:
 
     try:
         # Validate request
-        request = ModuleDetailsRequest(module_id=module_id, version=version)
+        request = ModuleDetailsRequest(module_id=module_id)
 
         # Import here to avoid circular imports
         from .tools.details import get_module_details_impl
@@ -282,7 +281,7 @@ async def get_module_details(module_id: str, version: str = "latest") -> str:
         log_tool_execution(
             logger,
             "get_module_details",
-            {"module_id": module_id, "version": version},
+            {"module_id": module_id},
             duration_ms,
             success=False,
             error="validation_error",
@@ -294,7 +293,7 @@ async def get_module_details(module_id: str, version: str = "latest") -> str:
         log_tool_execution(
             logger,
             "get_module_details",
-            {"module_id": module_id, "version": version},
+            {"module_id": module_id},
             duration_ms,
             success=False,
         )
@@ -305,7 +304,7 @@ async def get_module_details(module_id: str, version: str = "latest") -> str:
         log_tool_execution(
             logger,
             "get_module_details",
-            {"module_id": module_id, "version": version},
+            {"module_id": module_id},
             duration_ms,
             success=False,
             error=str(e),
@@ -315,7 +314,7 @@ async def get_module_details(module_id: str, version: str = "latest") -> str:
 
 
 @mcp.tool()
-async def list_content(module_id: str, version: str = "latest") -> str:
+async def list_content(module_id: str) -> str:
     """
     Discover available examples and repository structure - FIRST step in examples workflow.
 
@@ -341,8 +340,7 @@ async def list_content(module_id: str, version: str = "latest") -> str:
     - Use descriptions to select the single most relevant example
 
     Args:
-        module_id: Full module identifier (e.g., "terraform-ibm-modules/vpc/ibm")
-        version: Git tag/branch to scan (default: "latest")
+        module_id: Full module identifier (e.g., "terraform-ibm-modules/vpc/ibm" or "terraform-ibm-modules/vpc/ibm/1.2.3")
 
     Returns:
         Plain text with markdown formatted content listing organized by category
@@ -351,7 +349,7 @@ async def list_content(module_id: str, version: str = "latest") -> str:
 
     try:
         # Validate request
-        request = ListContentRequest(module_id=module_id, version=version)
+        request = ListContentRequest(module_id=module_id)
 
         # Import here to avoid circular imports
         from .tools.list_content import list_content_impl
@@ -376,7 +374,7 @@ async def list_content(module_id: str, version: str = "latest") -> str:
         log_tool_execution(
             logger,
             "list_content",
-            {"module_id": module_id, "version": version},
+            {"module_id": module_id},
             duration_ms,
             success=False,
             error="validation_error",
@@ -388,7 +386,7 @@ async def list_content(module_id: str, version: str = "latest") -> str:
         log_tool_execution(
             logger,
             "list_content",
-            {"module_id": module_id, "version": version},
+            {"module_id": module_id},
             duration_ms,
             success=False,
         )
@@ -399,7 +397,7 @@ async def list_content(module_id: str, version: str = "latest") -> str:
         log_tool_execution(
             logger,
             "list_content",
-            {"module_id": module_id, "version": version},
+            {"module_id": module_id},
             duration_ms,
             success=False,
             error=str(e),
@@ -414,7 +412,6 @@ async def get_content(
     path: str = "",
     include_files: str | list[str] | None = None,
     exclude_files: str | list[str] | None = None,
-    version: str = "latest",
 ) -> str:
     """
     Retrieve source code, examples, solutions from GitHub repositories with glob pattern filtering.
@@ -426,11 +423,10 @@ async def get_content(
     - Examples: path="examples/basic", include_files=["*.tf"]
 
     Args:
-        module_id: Full module identifier (e.g., "terraform-ibm-modules/vpc/ibm")
+        module_id: Full module identifier (e.g., "terraform-ibm-modules/vpc/ibm" or "terraform-ibm-modules/vpc/ibm/1.2.3")
         path: Specific path: "" (root), "examples/basic", "modules/vpc"
         include_files: Glob patterns for files to include (e.g., ["*.tf"], ["*.md"])
         exclude_files: Glob patterns for files to exclude (e.g., ["*test*"])
-        version: Git tag/branch to fetch from (default: "latest")
 
     Returns:
         Plain text with markdown formatted content
@@ -452,7 +448,6 @@ async def get_content(
             path=path,
             include_files=sanitized_include_files,
             exclude_files=sanitized_exclude_files,
-            version=version,
         )
 
         # Import here to avoid circular imports
@@ -483,7 +478,6 @@ async def get_content(
                 "path": path,
                 "include_files": include_files,
                 "exclude_files": exclude_files,
-                "version": version,
             },
             duration_ms,
             success=False,
@@ -501,7 +495,6 @@ async def get_content(
                 "path": path,
                 "include_files": include_files,
                 "exclude_files": exclude_files,
-                "version": version,
             },
             duration_ms,
             success=False,
@@ -518,7 +511,6 @@ async def get_content(
                 "path": path,
                 "include_files": include_files,
                 "exclude_files": exclude_files,
-                "version": version,
             },
             duration_ms,
             success=False,