feat: self-request as reviewer before posting #41

Merged
rodin merged 2 commits from feat/35-self-request-reviewer into main 2026-05-02 19:11:15 +00:00
Owner

Closes #35

Before posting a review, the bot discovers its own Gitea login via GET /user and calls POST /requested_reviewers to 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 login
  • gitea.Client.RequestReviewer() — adds a user as requested reviewer
  • Self-request happens before PostReview so the review satisfies the request
  • Both failures are non-fatal (warn + continue)
Closes #35 Before posting a review, the bot discovers its own Gitea login via `GET /user` and calls `POST /requested_reviewers` to 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 login - `gitea.Client.RequestReviewer()` — adds a user as requested reviewer - Self-request happens before `PostReview` so the review satisfies the request - Both failures are non-fatal (warn + continue)
rodin added 1 commit 2026-05-02 19:05:04 +00:00
feat: self-request as reviewer before posting
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 48s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 58s
c39845ca03
Closes #35

Before posting a review, the bot:
1. Discovers its own Gitea login via GET /user
2. Calls POST /requested_reviewers to add itself

This ensures the bot appears in the required-reviewers list without
manual configuration on the repo. The call is idempotent (no-op if
already requested).

Both failures are non-fatal (warn + continue) — the review still posts
even if the self-request fails.
gpt-review-bot approved these changes 2026-05-02 19:05:40 +00:00
Dismissed
gpt-review-bot left a comment
First-time contributor

Original review

Supersededsee 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 c39845ca

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/41#pullrequestreview-659) for up-to-date findings. <details><summary>Previous findings (commit c39845ca)</summary> # 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* <!-- review-bot:gpt --> --- *Evaluated against c39845ca* </details> <!-- review-bot:gpt -->
security-review-bot requested review from security-review-bot 2026-05-02 19:06:06 +00:00
security-review-bot approved these changes 2026-05-02 19:06:06 +00:00
Dismissed
security-review-bot left a comment
Collaborator

Security 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

# Severity File Line Finding
1 [MINOR] gitea/client.go 498 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.

Recommendation

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

# Security 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 | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `gitea/client.go` | 498 | 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. | ## Recommendation **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* <!-- review-bot:security --> --- *Evaluated against c39845ca*
@@ -463,0 +495,4 @@
}
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data))
if err != nil {
Collaborator

[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.

**[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.
sonnet-review-bot approved these changes 2026-05-02 19:06:17 +00:00
sonnet-review-bot left a comment
First-time contributor

Original review

Supersededsee 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

# Severity File Line Finding
1 [MINOR] gitea/client.go 506 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.
2 [MINOR] gitea/client.go 460 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.

Recommendation

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

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/41#pullrequestreview-662) for up-to-date findings. <details><summary>Previous findings (commit c39845ca)</summary> # 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 | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `gitea/client.go` | 506 | 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. | | 2 | [MINOR] | `gitea/client.go` | 460 | 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. | ## Recommendation **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* <!-- review-bot:sonnet --> --- *Evaluated against c39845ca* </details> <!-- review-bot:sonnet -->
@@ -460,3 +460,56 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID
}
First-time contributor

[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.

**[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)
First-time contributor

[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.

**[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.
rodin added 1 commit 2026-05-02 19:09:28 +00:00
fix: address review findings on RequestReviewer
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 35s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m3s
2507ee22e7
- Accept 204 No Content as success (idempotent operations)
- Truncate error response body to 256 bytes (prevent log leakage)
- Add unit tests for GetAuthenticatedUser and RequestReviewer
gpt-review-bot approved these changes 2026-05-02 19:10:03 +00:00
gpt-review-bot left a comment
First-time contributor

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 2507ee22

# 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* <!-- review-bot:gpt --> --- *Evaluated against 2507ee22*
security-review-bot requested review from security-review-bot 2026-05-02 19:10:17 +00:00
security-review-bot approved these changes 2026-05-02 19:10:17 +00:00
security-review-bot left a comment
Collaborator

Security 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 2507ee22

# Security 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* <!-- review-bot:security --> --- *Evaluated against 2507ee22*
sonnet-review-bot approved these changes 2026-05-02 19:10:47 +00:00
sonnet-review-bot left a comment
First-time contributor

Sonnet 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

# Severity File Line Finding
1 [MINOR] gitea/client.go 460 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.
2 [NIT] gitea/client.go 460 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.

Recommendation

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

# Sonnet 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 | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `gitea/client.go` | 460 | 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. | | 2 | [NIT] | `gitea/client.go` | 460 | 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. | ## Recommendation **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* <!-- review-bot:sonnet --> --- *Evaluated against 2507ee22*
@@ -460,3 +460,56 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID
}
First-time contributor

[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.

**[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.
First-time contributor

[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.

**[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.
rodin merged commit d30f3d4278 into main 2026-05-02 19:11:15 +00:00
rodin deleted branch feat/35-self-request-reviewer 2026-05-02 19:11:16 +00:00
Sign in to join this conversation.
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#41