Merge "Add /a/ prefix to Gerrit API URLs for authenticated access"

Author: Yevhenii-Yatchenko

gerrit_mcp_server/gerrit_urls.py

@@ -34,6 +34,10 @@ def get_curl_command_for_gerrit_url(
     stripped_gerrit_base_url = (
         gerrit_base_url.replace("https://", "").replace("http://", "").rstrip("/")
     )
+    # Strip /a suffix that may have been added by _normalize_gerrit_url
+    # for authenticated access, so we can match against configured host URLs.
+    if stripped_gerrit_base_url.endswith("/a"):
+        stripped_gerrit_base_url = stripped_gerrit_base_url[:-2]
 
     for host in gerrit_hosts:
         internal_url = (

gerrit_mcp_server/main.py

@@ -119,6 +119,16 @@ def _get_gerrit_base_url(gerrit_base_url: Optional[str] = None) -> str:
     )
 
 
+def _apply_authenticated_prefix(url: str, host: Dict[str, Any]) -> str:
+    """Appends /a to the URL if the host's auth type requires it for API access."""
+    auth_type = host.get("authentication", {}).get("type")
+
+    if auth_type in ("http_basic", "git_cookies"):
+        return url + "/a"
+
+    return url
+
+
 def _normalize_gerrit_url(url: str, gerrit_hosts: List[Dict[str, Any]]) -> str:
     """Normalizes a Gerrit URL based on the mappings in the provided gerrit_hosts."""
 
@@ -127,6 +137,7 @@ def _normalize_gerrit_url(url: str, gerrit_hosts: List[Dict[str, Any]]) -> str:
     stripped_original_url = original_url.replace("https://", "").replace("http://", "")
 
     normalized_url = url  # Default to original if no match found
+    matched_host = None
 
     for host in gerrit_hosts:
         internal_url = host.get("internal_url")
@@ -152,6 +163,7 @@ def _normalize_gerrit_url(url: str, gerrit_hosts: List[Dict[str, Any]]) -> str:
                 normalized_url = external_url
             elif internal_url:
                 normalized_url = internal_url
+            matched_host = host
             break  # Found a match, so we can exit the loop.
 
     # Ensure https, then strip trailing slash
@@ -162,7 +174,12 @@ def _normalize_gerrit_url(url: str, gerrit_hosts: List[Dict[str, Any]]) -> str:
     elif normalized_url.startswith("http://"):
         normalized_url = normalized_url.replace("http://", "https://")
 
-    return normalized_url.rstrip("/")
+    normalized_url = normalized_url.rstrip("/")
+
+    if matched_host:
+        return _apply_authenticated_prefix(normalized_url, matched_host)
+
+    return normalized_url
 
 
 async def run_curl(args: List[str], gerrit_base_url: str) -> str:
@@ -1178,7 +1195,7 @@ async def post_review_comment(
     }
     if labels:
         payload["labels"] = labels
-    
+
     args = _create_post_args(url, payload)
 
     try:

tests/unit/test_gerrit_config.py

@@ -95,5 +95,70 @@ def test_normalize_gerrit_url_with_mixed_mappings(self, mock_load_config):
         )
 
 
+    @patch("gerrit_mcp_server.main.load_gerrit_config")
+    def test_normalize_gerrit_url_adds_a_prefix_for_http_basic(self, mock_load_config):
+        mock_load_config.return_value = {
+            "gerrit_hosts": [
+                {
+                    "name": "GerritHub",
+                    "external_url": "https://review.gerrithub.io/",
+                    "authentication": {"type": "http_basic", "username": "u", "auth_token": "t"},
+                }
+            ]
+        }
+        gerrit_hosts = mock_load_config.return_value["gerrit_hosts"]
+        self.assertEqual(
+            _normalize_gerrit_url("review.gerrithub.io", gerrit_hosts), "https://review.gerrithub.io/a"
+        )
+
+    @patch("gerrit_mcp_server.main.load_gerrit_config")
+    def test_normalize_gerrit_url_adds_a_prefix_for_git_cookies(self, mock_load_config):
+        mock_load_config.return_value = {
+            "gerrit_hosts": [
+                {
+                    "name": "Self-hosted",
+                    "external_url": "https://gerrit.example.com/",
+                    "authentication": {"type": "git_cookies", "gitcookies_path": "~/.gitcookies"},
+                }
+            ]
+        }
+        gerrit_hosts = mock_load_config.return_value["gerrit_hosts"]
+        self.assertEqual(
+            _normalize_gerrit_url("gerrit.example.com", gerrit_hosts), "https://gerrit.example.com/a"
+        )
+
+    @patch("gerrit_mcp_server.main.load_gerrit_config")
+    def test_normalize_gerrit_url_no_a_prefix_for_gob_curl(self, mock_load_config):
+        mock_load_config.return_value = {
+            "gerrit_hosts": [
+                {
+                    "name": "Google",
+                    "external_url": "https://fuchsia-review.googlesource.com/",
+                    "authentication": {"type": "gob_curl"},
+                }
+            ]
+        }
+        gerrit_hosts = mock_load_config.return_value["gerrit_hosts"]
+        self.assertEqual(
+            _normalize_gerrit_url("fuchsia-review.googlesource.com", gerrit_hosts),
+            "https://fuchsia-review.googlesource.com",
+        )
+
+    @patch("gerrit_mcp_server.main.load_gerrit_config")
+    def test_normalize_gerrit_url_no_a_prefix_without_authentication(self, mock_load_config):
+        mock_load_config.return_value = {
+            "gerrit_hosts": [
+                {
+                    "name": "NoAuth",
+                    "external_url": "https://noauth.gerrit.com/",
+                }
+            ]
+        }
+        gerrit_hosts = mock_load_config.return_value["gerrit_hosts"]
+        self.assertEqual(
+            _normalize_gerrit_url("noauth.gerrit.com", gerrit_hosts), "https://noauth.gerrit.com"
+        )
+
+
 if __name__ == "__main__":
     unittest.main()

tests/unit/test_gerrit_urls.py

@@ -81,6 +81,26 @@ def test_dispatches_to_gitcookies(self, mock_get_auth):
         mock_get_auth.assert_called_once_with(url, auth_config)
         self.assertEqual(command, ["curl", "-b", "cookie", "-L"])
 
+    @patch("gerrit_mcp_server.gerrit_auth._get_auth_for_http_basic")
+    def test_dispatches_correctly_with_a_prefix_in_url(self, mock_get_auth):
+        """Tests that the dispatcher matches the host when the URL contains /a suffix."""
+        auth_config = {"type": "http_basic", "username": "test", "auth_token": "token"}
+        mock_get_auth.return_value = ["curl", "--user", "test:token", "-L"]
+        config = {
+            "gerrit_hosts": [
+                {
+                    "name": "GerritHub",
+                    "external_url": "https://review.gerrithub.io/",
+                    "authentication": auth_config,
+                }
+            ]
+        }
+        command = gerrit_urls.get_curl_command_for_gerrit_url(
+            "https://review.gerrithub.io/a", config
+        )
+        mock_get_auth.assert_called_once_with(auth_config)
+        self.assertEqual(command, ["curl", "--user", "test:token", "-L"])
+
     def test_no_matching_host_raises_error(self):
         """Tests that a ValueError is raised when no matching host is found."""
         config = {"gerrit_hosts": []}