feat: self-request as reviewer before posting #41
Reference in New Issue
Block a user
Delete Branch "feat/35-self-request-reviewer"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #35
Before posting a review, the bot discovers its own Gitea login via
GET /userand callsPOST /requested_reviewersto add itself to the PR.Why
With branch protection requiring specific bot accounts as reviewers, the bot needs to be in the requested-reviewers list. Previously this required manual repo configuration. Now it's automatic and idempotent.
Changes
gitea.Client.GetAuthenticatedUser()— discovers the token's logingitea.Client.RequestReviewer()— adds a user as requested reviewerPostReviewso the review satisfies the requestOriginal reviewSuperseded — see current review for up-to-date findings.
Previous findings (commit
c39845ca)Gpt Review
Summary
This change is well-executed: it introduces the ability for the bot to discover its own identity and request itself as a reviewer, which is vital for compliance with branch-protection rules. The implementation is robust, non-fatal on error, and idiomatic.
Recommendation
APPROVE — The change is sound: it introduces minimal, focused new methods in the Gitea client and calls them in the correct order before posting reviews. Error handling is non-fatal (warns and continues), matching expected bot behavior. Code style, concurrency safety, and error handling are idiomatic and conform to project/repo patterns. No breaking changes or issues found. Ship it.
Review by gpt
Evaluated against
c39845caSecurity Review
Summary
The added self-request-as-reviewer feature uses authenticated Gitea API calls with proper context, path escaping, and error handling. No exploitable security issues are evident, and CI has passed.
Findings
gitea/client.goRecommendation
APPROVE — The implementation appears secure and correct for the intended feature. The API interactions use context-aware requests, URL path escaping, and do not expose secrets. As a minor hardening measure, consider truncating or sanitizing HTTP response bodies included in error messages to limit potential information leakage in logs. Otherwise, the changes are suitable to merge.
Review by security
Evaluated against
c39845ca@@ -463,0 +495,4 @@}req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data))if err != nil {[MINOR] On reviewer request failure, the full HTTP response body is included in the error message, which may leak internal server details into logs. Consider truncating or redacting the body before logging to reduce potential information exposure.
Original reviewSuperseded — see current review for up-to-date findings.
Previous findings (commit
c39845ca)Sonnet Review
Summary
The changes cleanly add self-requesting as a reviewer and extend the Gitea client with well-structured, idiomatic methods. Error handling and logging are appropriate, and CI passed. A couple of minor robustness/testing points could be improved.
Findings
gitea/client.gogitea/client.goRecommendation
APPROVE — Overall, the feature is implemented correctly and safely: self-requesting the authenticated user as a reviewer happens before posting the review, with non-fatal error handling and good logging. The Gitea client additions are idiomatic and use proper error wrapping. To further harden the implementation, consider treating HTTP 204 as a successful response in RequestReviewer to handle idempotent server behavior, and add unit tests for the newly exported client methods to align with repository testing conventions. With these minor improvements, the change set is solid.
Review by sonnet
Evaluated against
c39845ca@@ -460,3 +460,56 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID}[MINOR] New exported methods (GetAuthenticatedUser and RequestReviewer) were added without accompanying tests. Repository conventions state to test every exported function; adding unit tests would improve coverage and guard against API regressions.
@@ -463,0 +503,4 @@resp, err := c.http.Do(req)if err != nil {return fmt.Errorf("request reviewer: %w", err)[MINOR] RequestReviewer treats only 200 OK and 201 Created as success. Consider accepting 204 No Content as success as well, since some APIs return 204 for idempotent operations.
Gpt Review
Summary
This change adds the ability for the bot to self-request as a reviewer before posting its review, with robust handling for errors and corresponding tests. The implementation and test coverage are thorough and idiomatic.
Recommendation
APPROVE — All aspects of the change are solid: the self-request logic is non-fatal and logs issues, the new client methods are correct and robust, and new tests for both reviewer self-request and user discovery are present. The code is idiomatic and all conventions are respected. Good work!
Review by gpt
Evaluated against
2507ee22Security Review
Summary
Security-focused review finds the new self-request and Gitea client methods implemented safely with proper URL escaping, bounded error reads, and contextual error wrapping. No sensitive data is logged and operations are bounded by context and HTTP client timeouts.
Recommendation
APPROVE — The additions to request the authenticated user and self-request as a reviewer are implemented securely and defensively. URL/path parameters are escaped, JSON payloads are constructed safely, responses on error paths are size-limited, and failures are non-fatal as intended. CI passed and tests cover the new methods. No changes requested.
Review by security
Evaluated against
2507ee22Sonnet Review
Summary
Change cleanly adds self-requesting as a reviewer before posting and introduces well-tested Gitea client methods to support it. Implementation is idiomatic, error-tolerant, and preserves existing behavior with clear logging.
Findings
gitea/client.gogitea/client.goRecommendation
APPROVE — The additions are well-structured and tested, and CI is green. The self-request flow is placed appropriately before posting the review and failures are correctly treated as non-fatal. To improve resilience, consider treating any 2xx status as success (or explicitly include 202) in RequestReviewer, and align the idempotency comment with the exact behavior or accepted statuses. Otherwise, the changes are solid and ready to merge.
Review by sonnet
Evaluated against
2507ee22@@ -460,3 +460,56 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID}[MINOR] RequestReviewer treats only 200/201/204 as success. For robustness and future compatibility, consider accepting any 2xx status (or explicitly include 202 Accepted) to avoid false negatives if the server returns a different success code.
[NIT] The comment states the RequestReviewer call is idempotent, but if the API returned a non-2xx for 'already requested' (e.g., 409/422), the method would currently return an error. Consider broadening accepted statuses or documenting expected server behavior more precisely.