fix(gerrit): raise HTTPException instead of returning it

[gvago]

Summary

• Change return HTTPException(...) to raise HTTPException(...) in gerrit_server.py
• Without this fix, FastAPI returns 200 OK with the exception serialized as JSON instead of a proper 400 error

Test plan

• Verify gerrit server returns 400 on invalid requests

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix Gerrit server to raise HTTPException properly

🐞 Bug fix

Walkthroughs

Description

• Changed return HTTPException(...) to raise HTTPException(...)
• Fixes FastAPI returning 200 OK with serialized exception
• Ensures proper 400 error response for invalid requests

Diagram

flowchart LR
  A["Invalid request<br/>msg is required"] -->|Before: return| B["200 OK<br/>Exception as JSON"]
  A -->|After: raise| C["400 Bad Request<br/>Proper error response"]

Loading

File Changes

1. pr_agent/servers/gerrit_server.py 🐞 Bug fix +1/-1

Raise instead of return HTTPException

• Changed return HTTPException(...) to raise HTTPException(...) in handle_gerrit_request
 function
• Affects validation of msg parameter for ask command
• Ensures FastAPI properly handles the exception with correct HTTP status code

pr_agent/servers/gerrit_server.py

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)

🐞\ ≡ Correctness (1)

1. Whitespace msg bypass 🐞 ≡

Description

In handle_gerrit_request, the validation checks if not item.msg before stripping whitespace, so
a whitespace-only msg passes validation but becomes an empty command via item.msg.strip(). This
can lead to PRAgent treating the request as an unknown command (returning False) while the
endpoint completes without raising an HTTP error.

Code

pr_agent/servers/gerrit_server.py[R40-45]

    if action == Action.ask:
        if not item.msg:
-            return HTTPException(
+            raise HTTPException(
                status_code=400,
                detail="msg is required for ask command"
            )

Evidence

The endpoint validates the raw item.msg but later strips it when building the PRAgent request,
enabling whitespace-only messages to slip past the ask guard. PRAgent normalizes the action by
stripping leading /; if the action becomes empty, it is treated as unknown and the call returns
False (with no exception thrown by this path).

pr_agent/servers/gerrit_server.py[40-49]
pr_agent/agent/pr_agent.py[59-104]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`handle_gerrit_request` validates `item.msg` before trimming whitespace, but later uses `item.msg.strip()` to build the PRAgent request. This allows whitespace-only messages to bypass the `ask` validation and become an empty/unknown command downstream.

### Issue Context
For `Action.ask`, the handler intends to reject missing/empty messages, but should treat whitespace-only input as empty too.

### Fix Focus Areas
- pr_agent/servers/gerrit_server.py[40-49]

### Suggested change
Update the check to validate the stripped value, e.g.:
- `if not item.msg or not item.msg.strip(): raise HTTPException(...)`
(or equivalently `if not item.msg.strip(): ...` if `msg` is guaranteed non-None).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more