Sonnet Review Bot sonnet-review-bot
  • Joined on 2026-04-24
sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:48:44 +00:00
feat: inline review comments on specific lines

[MINOR] shouldEscalate relies on detecting a 'sentinel' substring in review bodies to identify sibling bot reviews. An untrusted user could spoof the sentinel in their own review, potentially causing unintended escalation. Consider additionally verifying the matched review's user login matches the bot's account (derived independently from the API) before treating it as a sibling.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:48:44 +00:00
feat: inline review comments on specific lines

[MINOR] PostReview constructs the reviews API URL using owner and repo without url.PathEscape, which can lead to malformed paths if these contain unexpected characters or slashes. While inputs are typically controlled, escaping path segments (as done elsewhere in this client) is a defense-in-depth improvement.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:48:38 +00:00
feat: inline review comments on specific lines

[MAJOR] Escalation now happens before posting and depends on deriving ownLogin from an existing review containing the sentinel. On the first run (no existing review with this sentinel), ownLogin is empty and escalation is skipped, so an APPROVED review may be posted even if a sibling bot (same token) already has REQUEST_CHANGES. Previously this was handled by escalating after posting using the posted user login.

sonnet-review-bot suggested changes for rodin/review-bot#26 2026-05-02 05:48:38 +00:00
feat: inline review comments on specific lines

Sonnet Review

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:48:38 +00:00
feat: inline review comments on specific lines

[MINOR] EditComment marshals the payload with json.Marshal but ignores the error (data, _ := json.Marshal(payload)). This can mask unexpected failures; handle and return the marshal error similar to PostReview.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:48:38 +00:00
feat: inline review comments on specific lines

[NIT] In reviewUnchanged, the condition existingEvent == r.State is redundant since existingEvent is assigned from r.State. Simplify to r.State == newEvent && r.Body == newBody.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:19:22 +00:00
feat: inline review comments on specific lines

[NIT] Hunk header parsing uses strings.Split(line, "+") and then trims at comma/space. While adequate for typical diffs, consider a more targeted parse (e.g., locating the first '+' between the '@@' markers) to be more robust against unusual hunk headers.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:19:22 +00:00
feat: inline review comments on specific lines

[NIT] In reviewUnchanged, the check existingEvent == r.State is redundant since existingEvent is assigned from r.State; simplify the condition to if r.State == newEvent && r.Body == newBody for clarity.

sonnet-review-bot approved rodin/review-bot#26 2026-05-02 05:18:55 +00:00
feat: inline review comments on specific lines

Security Review

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:12:49 +00:00
feat: inline review comments on specific lines

[NIT] The set of line numbers is represented as map[int]bool. Using map[int]struct{} is slightly more idiomatic and marginally more memory-efficient for set semantics.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:12:49 +00:00
feat: inline review comments on specific lines

[MINOR] When extracting the current file from the '+++ b/...' header, consider trimming any trailing '\r' to be robust against CRLF line endings, e.g., strings.TrimSuffix(currentFile, "\r"). Otherwise, Contains may fail to match file names on diffs with CRLF.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:12:02 +00:00
feat: inline review comments on specific lines

[MINOR] In PostReview, the request URL is built with owner and repo interpolated without path escaping (fmt.Sprintf(.../%s/%s/...)). If these values were ever untrusted, this could allow path injection. Other methods (e.g., ListReviews, DeleteReview) use url.PathEscape for owner and repo. For consistency and defense-in-depth, apply PathEscape to owner and repo here as well.

sonnet-review-bot approved rodin/review-bot#26 2026-05-02 05:12:02 +00:00
feat: inline review comments on specific lines

Security Review

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:01:14 +00:00
feat: inline review comments on specific lines

[NIT] Consider documenting that inline comment bodies are Markdown-formatted (bold severity) to align with Gitea rendering expectations, or consider a consistent plain-text format if Markdown rendering is not guaranteed.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:01:14 +00:00
feat: inline review comments on specific lines

[MINOR] Lines such as "\ No newline at end of file" (which can appear in unified diffs) aren't filtered explicitly; while currently ignored by the prefix checks, a comment noting this or an explicit skip can make intent clearer and avoid accidental inclusion if future logic changes.

sonnet-review-bot commented on pull request rodin/review-bot#26 2026-05-02 05:01:14 +00:00
feat: inline review comments on specific lines

[MINOR] Hunk header parsing assumes a comma-delimited "+new,count" form; headers like "@@ -1 +1 @@" (no comma) or trailing text after the number may cause Atoi to fail. Consider trimming non-digits (up to space or '@@') or handling the no-comma case to be more robust.

sonnet-review-bot approved rodin/review-bot#26 2026-05-02 05:00:27 +00:00
feat: inline review comments on specific lines

Security Review