fix(gerrit): use action parameter for dispatch and clean up temp repos

pr_agent/git_providers/gerrit_provider.py

@@ -387,9 +387,30 @@ def publish_labels(self, labels):
         # but required by the interface
         pass
 
+    def cleanup(self):
+        """Remove the temporary cloned repository from disk."""
+        if self.repo_path and pathlib.Path(self.repo_path).exists():
+            try:
+                shutil.rmtree(self.repo_path, ignore_errors=True)
+                get_logger().info("Cleaned up temp repo at %s", self.repo_path)
+            except (OSError, PermissionError) as e:
+                get_logger().warning(
+                    "Failed to clean up temp repo at %s: %s",
+                    self.repo_path, e
+                )
+
+    def __del__(self):
+        """Safety net: clean up temp repo if cleanup() was not called."""
+        try:
+            self.cleanup()
+        except Exception:
+            pass
+
     def remove_initial_comment(self):
-        # remove repo, cloned in previous steps
-        # shutil.rmtree(self.repo_path)
+        # Do NOT call cleanup() here — this method is invoked during the
+        # request lifecycle while the cloned repo is still needed by
+        # subsequent commands.  Actual cleanup happens in the server's
+        # finally block and in __del__ as a safety net.
         pass
 
     def remove_comment(self, comment):

pr_agent/servers/gerrit_server.py

@@ -11,6 +11,7 @@
 
 from pr_agent.agent.pr_agent import PRAgent
 from pr_agent.config_loader import get_settings, global_settings
+from pr_agent.git_providers.gerrit_provider import GerritProvider
 from pr_agent.log import get_logger, setup_logger
 
 setup_logger()
@@ -29,24 +30,54 @@ class Action(str, Enum):
 class Item(BaseModel):
     refspec: str
     project: str
-    msg: str
+    msg: str = ""
 
 
 @router.post("/api/v1/gerrit/{action}")
 async def handle_gerrit_request(action: Action, item: Item):
     get_logger().debug("Received a Gerrit request")
     context["settings"] = copy.deepcopy(global_settings)
 
+    # For the "ask" action, the question must come from item.msg.
+    # For all other actions, use the action path parameter as the command.
     if action == Action.ask:
         if not item.msg:
-            return HTTPException(
+            raise HTTPException(
                 status_code=400,
                 detail="msg is required for ask command"
             )
-    await PRAgent().handle_request(
-        f"{item.project}:{item.refspec}",
-        f"/{item.msg.strip()}"
-    )
+        command = f"/{action.value} {item.msg.strip()}"
+    else:
+        command = f"/{action.value}"
+
+    try:
+        await PRAgent().handle_request(
+            f"{item.project}:{item.refspec}",
+            command
+        )
+    finally:
+        # Clean up the cloned temp repo created by GerritProvider.
+        # The provider is cached in the starlette context during
+        # get_git_provider_with_context().
+        #
+        # We guard against two failure modes:
+        #   1. The starlette context is inaccessible (e.g. middleware not
+        #      active) — caught by the outer try/except.
+        #   2. The provider was never stored in the context (e.g. an error
+        #      occurred before get_git_provider_with_context ran) — the
+        #      dict will simply be empty, and the GerritProvider.__del__
+        #      safety net handles cleanup on garbage collection.
+        try:
+            git_providers = context.get("git_provider", {})
+            if isinstance(git_providers, dict):
+                for provider in git_providers.values():
+                    if isinstance(provider, GerritProvider):
+                        provider.cleanup()
+        except (LookupError, RuntimeError):
+            get_logger().debug(
+                "Could not retrieve GerritProvider for cleanup; "
+                "temp directory will be cleaned up by __del__"
+            )
 
 
 async def get_body(request):