Compare commits
19 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 489457c184 | |||
| 25d1a670bf | |||
| 80a9a7675b | |||
| 8d8a249481 | |||
| a0fd882b0d | |||
| d4bf13eeab | |||
| 23443ef378 | |||
| bc5a4a1dcd | |||
| d30f3d4278 | |||
| 2507ee22e7 | |||
| c39845ca03 | |||
| cd601bdcf4 | |||
| 50091941e1 | |||
| ed06cdd942 | |||
| ed69d26e87 | |||
| da586a512a | |||
| f6baa41b2c | |||
| ecbae332f4 | |||
| fdd75699d9 |
+122
-66
@@ -67,7 +67,6 @@ func main() {
|
||||
patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
|
||||
patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo")
|
||||
dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
|
||||
updateExisting := flag.Bool("update-existing", envOrDefaultBool("UPDATE_EXISTING", true), "Delete previous review from same bot before posting (default true)")
|
||||
llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
|
||||
llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
|
||||
llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai or anthropic")
|
||||
@@ -279,6 +278,16 @@ func main() {
|
||||
|
||||
// Step 10: Format and post review
|
||||
reviewBody := review.FormatMarkdown(result, *reviewerName)
|
||||
|
||||
// Add commit footer so readers know which commit was evaluated
|
||||
if pr.Head.Sha != "" {
|
||||
shortSHA := pr.Head.Sha
|
||||
if len(shortSHA) > 8 {
|
||||
shortSHA = shortSHA[:8]
|
||||
}
|
||||
reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA)
|
||||
}
|
||||
|
||||
event := review.GiteaEvent(result.Verdict)
|
||||
|
||||
if *dryRun {
|
||||
@@ -307,61 +316,36 @@ func main() {
|
||||
}
|
||||
|
||||
// --- Review update strategy ---
|
||||
// 1. No existing review → POST new
|
||||
// 2. Existing review, same state → PATCH body in place (preserves threads)
|
||||
// 3. Existing review, state change → PATCH old to "Superseded", POST new
|
||||
if *updateExisting && *reviewerName != "" {
|
||||
// 1. POST new review first (gets non-stale approval badge on HEAD)
|
||||
// 2. Then supersede old review with link to the new one
|
||||
// Order matters: post first so we have the new review's URL for the supersede message.
|
||||
var oldReviews []gitea.Review
|
||||
if *reviewerName != "" {
|
||||
existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
|
||||
if err != nil {
|
||||
slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
|
||||
} else {
|
||||
// Detect shared-token misconfiguration: if detected, skip all
|
||||
// update logic (PATCH/supersede) to avoid clobbering a sibling's review.
|
||||
sharedToken := hasSharedToken(existingReviews, sentinel)
|
||||
if sharedToken {
|
||||
slog.Warn("shared token mode: skipping update-in-place logic to avoid clobbering sibling review")
|
||||
if hasSharedToken(existingReviews, sentinel) {
|
||||
slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review")
|
||||
} else {
|
||||
existing := findOwnReview(existingReviews, sentinel)
|
||||
|
||||
if existing != nil {
|
||||
if reviewUnchanged(existingReviews, reviewBody, event, sentinel) {
|
||||
slog.Info("review unchanged from previous run; skipping to preserve threads", "pr", prNumber)
|
||||
return
|
||||
}
|
||||
|
||||
// Same state → PATCH in place
|
||||
if existing.State == event {
|
||||
commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
|
||||
if err != nil {
|
||||
slog.Warn("could not find review comment ID, falling back to new post", "error", err)
|
||||
} else {
|
||||
if err := giteaClient.EditComment(ctx, owner, repoName, commentID, reviewBody); err != nil {
|
||||
slog.Warn("could not edit review, falling back to new post", "comment_id", commentID, "error", err)
|
||||
} else {
|
||||
slog.Info("review updated in place", "comment_id", commentID, "pr", prNumber)
|
||||
return
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// State change → mark old as superseded, post new below
|
||||
commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
|
||||
if err != nil {
|
||||
slog.Warn("could not find old review comment ID", "error", err)
|
||||
} else {
|
||||
supersededBody := fmt.Sprintf("~~*This review has been superseded by a newer review below.*~~\n\n%s", sentinel)
|
||||
if err := giteaClient.EditComment(ctx, owner, repoName, commentID, supersededBody); err != nil {
|
||||
slog.Warn("could not mark old review as superseded", "comment_id", commentID, "error", err)
|
||||
} else {
|
||||
slog.Info("marked old review as superseded", "old_state", existing.State, "new_state", event, "pr", prNumber)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
oldReviews = findAllOwnReviews(existingReviews, sentinel)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// POST new review (first run, or state transition fallthrough)
|
||||
// Self-request as reviewer (ensures we appear in required-reviewer checks)
|
||||
authUser, err := giteaClient.GetAuthenticatedUser(ctx)
|
||||
if err != nil {
|
||||
slog.Warn("could not determine authenticated user for reviewer self-request", "error", err)
|
||||
} else if authUser != "" {
|
||||
if err := giteaClient.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
|
||||
slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
|
||||
} else {
|
||||
slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
|
||||
}
|
||||
}
|
||||
|
||||
// POST new review
|
||||
slog.Info("posting review", "event", event, "pr", prNumber)
|
||||
posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, inlineComments)
|
||||
if err != nil {
|
||||
@@ -370,6 +354,49 @@ func main() {
|
||||
}
|
||||
slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber)
|
||||
|
||||
// Supersede all old reviews with link to the new one
|
||||
if len(oldReviews) > 0 {
|
||||
newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*giteaURL, "/"), owner, repoName, prNumber, posted.ID)
|
||||
for _, oldReview := range oldReviews {
|
||||
cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
|
||||
if err != nil {
|
||||
slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
|
||||
continue
|
||||
}
|
||||
supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel)
|
||||
if err := giteaClient.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
|
||||
slog.Warn("could not mark old review as superseded", "review_id", oldReview.ID, "comment_id", cid, "error", err)
|
||||
continue
|
||||
}
|
||||
slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber)
|
||||
|
||||
// Resolve old review's inline comments
|
||||
oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
|
||||
if err != nil {
|
||||
slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
|
||||
continue
|
||||
}
|
||||
resolved, failed := 0, 0
|
||||
for _, c := range oldComments {
|
||||
if c.ID == 0 {
|
||||
continue
|
||||
}
|
||||
if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
|
||||
slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
|
||||
failed++
|
||||
} else {
|
||||
resolved++
|
||||
}
|
||||
}
|
||||
if resolved > 0 {
|
||||
slog.Info("resolved old inline comments", "review_id", oldReview.ID, "count", resolved, "pr", prNumber)
|
||||
}
|
||||
if failed > 0 {
|
||||
slog.Warn("some inline comments could not be resolved", "review_id", oldReview.ID, "failed", failed, "pr", prNumber)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
// fetchFileContext fetches the full content of modified files from the PR branch.
|
||||
@@ -526,22 +553,29 @@ func validateReviewerName(name string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// reviewUnchanged checks if an existing review with the same sentinel
|
||||
// already has identical body and state. Returns true if a re-post would
|
||||
// produce the same result (skip to preserve conversation threads).
|
||||
func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string) bool {
|
||||
for _, r := range reviews {
|
||||
if r.Stale {
|
||||
continue
|
||||
}
|
||||
if !strings.Contains(r.Body, sentinel) {
|
||||
continue
|
||||
}
|
||||
if r.State == newEvent && r.Body == newBody {
|
||||
return true
|
||||
}
|
||||
// buildSupersededBody creates the body for a superseded review: struck-through banner
|
||||
// with collapsed original content and the commit it was evaluated against.
|
||||
func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string) string {
|
||||
shortSHA := commitSHA
|
||||
if len(shortSHA) > 8 {
|
||||
shortSHA = shortSHA[:8]
|
||||
}
|
||||
return false
|
||||
var sb strings.Builder
|
||||
sb.WriteString("~~Original review~~\n\n")
|
||||
sb.WriteString("**Superseded** \u2014 [see current review](")
|
||||
sb.WriteString(newReviewURL)
|
||||
sb.WriteString(") for up-to-date findings.\n\n")
|
||||
if shortSHA != "" {
|
||||
sb.WriteString("<details><summary>Previous findings (commit ")
|
||||
sb.WriteString(shortSHA)
|
||||
sb.WriteString(")</summary>\n\n")
|
||||
} else {
|
||||
sb.WriteString("<details><summary>Previous findings</summary>\n\n")
|
||||
}
|
||||
sb.WriteString(originalBody)
|
||||
sb.WriteString("\n\n</details>\n\n")
|
||||
sb.WriteString(sentinel)
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
// hasSharedToken detects if another review-bot role posted under the same
|
||||
@@ -585,12 +619,34 @@ func extractSentinelName(body string) string {
|
||||
return rest[:end]
|
||||
}
|
||||
|
||||
// findOwnReview locates a review matching the given sentinel in its body.
|
||||
// findOwnReview locates the most recent non-superseded review matching the sentinel.
|
||||
func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
|
||||
var best *gitea.Review
|
||||
for i := range reviews {
|
||||
if strings.Contains(reviews[i].Body, sentinel) {
|
||||
return &reviews[i]
|
||||
if !strings.Contains(reviews[i].Body, sentinel) {
|
||||
continue
|
||||
}
|
||||
if strings.Contains(reviews[i].Body, "~~Original review~~") {
|
||||
continue
|
||||
}
|
||||
if best == nil || reviews[i].ID > best.ID {
|
||||
best = &reviews[i]
|
||||
}
|
||||
}
|
||||
return nil
|
||||
return best
|
||||
}
|
||||
|
||||
// findAllOwnReviews returns all non-superseded reviews matching the sentinel.
|
||||
func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
|
||||
var result []gitea.Review
|
||||
for i := range reviews {
|
||||
if !strings.Contains(reviews[i].Body, sentinel) {
|
||||
continue
|
||||
}
|
||||
if strings.Contains(reviews[i].Body, "~~Original review~~") {
|
||||
continue
|
||||
}
|
||||
result = append(result, reviews[i])
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
+92
-75
@@ -56,82 +56,52 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re
|
||||
return r
|
||||
}
|
||||
|
||||
func TestReviewUnchanged(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
existing []gitea.Review
|
||||
newBody string
|
||||
newEvent string
|
||||
sentinel string
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "no existing review",
|
||||
existing: nil,
|
||||
newBody: "new review",
|
||||
newEvent: "APPROVED",
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "identical body and state",
|
||||
existing: []gitea.Review{
|
||||
makeReview(100, "bot", "APPROVED", false, "same body\n<!-- review-bot:sonnet -->"),
|
||||
},
|
||||
newBody: "same body\n<!-- review-bot:sonnet -->",
|
||||
newEvent: "APPROVED",
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "same body but different state",
|
||||
existing: []gitea.Review{
|
||||
makeReview(100, "bot", "APPROVED", false, "body\n<!-- review-bot:sonnet -->"),
|
||||
},
|
||||
newBody: "body\n<!-- review-bot:sonnet -->",
|
||||
newEvent: "REQUEST_CHANGES",
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "different body same state",
|
||||
existing: []gitea.Review{
|
||||
makeReview(100, "bot", "APPROVED", false, "old body\n<!-- review-bot:sonnet -->"),
|
||||
},
|
||||
newBody: "new body\n<!-- review-bot:sonnet -->",
|
||||
newEvent: "APPROVED",
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "stale review with same body (should still post)",
|
||||
existing: []gitea.Review{
|
||||
makeReview(100, "bot", "APPROVED", true, "same\n<!-- review-bot:sonnet -->"),
|
||||
},
|
||||
newBody: "same\n<!-- review-bot:sonnet -->",
|
||||
newEvent: "APPROVED",
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "different sentinel (not our review)",
|
||||
existing: []gitea.Review{
|
||||
makeReview(100, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
|
||||
},
|
||||
newBody: "body\n<!-- review-bot:sonnet -->",
|
||||
newEvent: "APPROVED",
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := reviewUnchanged(tc.existing, tc.newBody, tc.newEvent, tc.sentinel)
|
||||
if got != tc.want {
|
||||
t.Errorf("reviewUnchanged() = %v, want %v", got, tc.want)
|
||||
}
|
||||
})
|
||||
func TestBuildSupersededBody(t *testing.T) {
|
||||
original := "# Review\n\nLooks good.\n\n<!-- review-bot:sonnet -->"
|
||||
sentinel := "<!-- review-bot:sonnet -->"
|
||||
newURL := "https://gitea.example.com/owner/repo/pulls/1#pullrequestreview-99"
|
||||
|
||||
result := buildSupersededBody(original, "abcdef1234567890", newURL, sentinel)
|
||||
|
||||
// Should contain the struck-through banner
|
||||
if !strings.Contains(result, "~~Original review~~") {
|
||||
t.Error("missing struck-through banner")
|
||||
}
|
||||
// Should contain superseded notice with link
|
||||
if !strings.Contains(result, "**Superseded**") {
|
||||
t.Error("missing superseded notice")
|
||||
}
|
||||
if !strings.Contains(result, "[see current review]("+newURL+")") {
|
||||
t.Error("missing link to new review")
|
||||
}
|
||||
// Should contain collapsed original
|
||||
if !strings.Contains(result, "<details>") {
|
||||
t.Error("missing details/collapse")
|
||||
}
|
||||
// Should contain short commit SHA
|
||||
if !strings.Contains(result, "abcdef12") {
|
||||
t.Error("missing short SHA")
|
||||
}
|
||||
// Should NOT contain full SHA
|
||||
if strings.Contains(result, "abcdef1234567890") {
|
||||
t.Error("should truncate SHA to 8 chars")
|
||||
}
|
||||
// Should contain the original body inside details
|
||||
if !strings.Contains(result, original) {
|
||||
t.Error("original body not preserved in collapsed section")
|
||||
}
|
||||
// Should end with sentinel
|
||||
if !strings.Contains(result, sentinel) {
|
||||
t.Error("missing sentinel")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildSupersededBodyShortSHA(t *testing.T) {
|
||||
// Short SHA should pass through without panic
|
||||
result := buildSupersededBody("body", "abc", "https://example.com/review", "<!-- review-bot:x -->")
|
||||
if !strings.Contains(result, "abc") {
|
||||
t.Error("short SHA not preserved")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -174,6 +144,32 @@ func TestFindOwnReview(t *testing.T) {
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
wantID: 20,
|
||||
},
|
||||
{
|
||||
name: "skips superseded review",
|
||||
reviews: []gitea.Review{
|
||||
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n<!-- review-bot:sonnet -->"),
|
||||
makeReview(20, "bot", "APPROVED", false, "fresh review\n<!-- review-bot:sonnet -->"),
|
||||
},
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
wantID: 20,
|
||||
},
|
||||
{
|
||||
name: "only superseded reviews exist",
|
||||
reviews: []gitea.Review{
|
||||
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
|
||||
},
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
wantNil: true,
|
||||
},
|
||||
{
|
||||
name: "picks highest ID among matches",
|
||||
reviews: []gitea.Review{
|
||||
makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
|
||||
makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
|
||||
},
|
||||
sentinel: "<!-- review-bot:sonnet -->",
|
||||
wantID: 50,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
@@ -845,3 +841,24 @@ func cleanEnv() []string {
|
||||
}
|
||||
return env
|
||||
}
|
||||
|
||||
func TestFindAllOwnReviews(t *testing.T) {
|
||||
reviews := []gitea.Review{
|
||||
{ID: 1, Body: "<!-- review-bot:sonnet -->\nfirst review"},
|
||||
{ID: 2, Body: "<!-- review-bot:gpt -->\nother bot"},
|
||||
{ID: 3, Body: "<!-- review-bot:sonnet -->\nsecond review"},
|
||||
{ID: 4, Body: "~~Original review~~\n<!-- review-bot:sonnet -->\nsuperseded"},
|
||||
{ID: 5, Body: "<!-- review-bot:sonnet -->\nthird review"},
|
||||
}
|
||||
|
||||
got := findAllOwnReviews(reviews, "<!-- review-bot:sonnet -->")
|
||||
if len(got) != 3 {
|
||||
t.Fatalf("findAllOwnReviews() returned %d, want 3", len(got))
|
||||
}
|
||||
wantIDs := []int64{1, 3, 5}
|
||||
for i, r := range got {
|
||||
if r.ID != wantIDs[i] {
|
||||
t.Errorf("got[%d].ID = %d, want %d", i, r.ID, wantIDs[i])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+179
-5
@@ -82,6 +82,7 @@ type ChangedFile struct {
|
||||
|
||||
// ReviewComment represents an inline comment to attach to a review.
|
||||
type ReviewComment struct {
|
||||
ID int64 `json:"id,omitempty"`
|
||||
Path string `json:"path"`
|
||||
NewPosition int64 `json:"new_position"`
|
||||
Body string `json:"body"`
|
||||
@@ -316,13 +317,14 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
|
||||
|
||||
// Review represents a pull request review from the Gitea API.
|
||||
type Review struct {
|
||||
ID int64 `json:"id"`
|
||||
Body string `json:"body"`
|
||||
User struct {
|
||||
ID int64 `json:"id"`
|
||||
Body string `json:"body"`
|
||||
User struct {
|
||||
Login string `json:"login"`
|
||||
} `json:"user"`
|
||||
State string `json:"state"`
|
||||
Stale bool `json:"stale"`
|
||||
State string `json:"state"`
|
||||
Stale bool `json:"stale"`
|
||||
CommitID string `json:"commit_id"`
|
||||
}
|
||||
|
||||
// ListReviews returns all reviews on a pull request.
|
||||
@@ -424,6 +426,68 @@ func (c *Client) GetTimelineReviewCommentID(ctx context.Context, owner, repo str
|
||||
return 0, fmt.Errorf("no timeline event found with sentinel")
|
||||
}
|
||||
|
||||
// GetTimelineReviewCommentIDForReview finds the timeline comment ID for a
|
||||
// specific review by matching its body content in the timeline.
|
||||
func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error) {
|
||||
// Use the reviews API to get the review body, then find in timeline
|
||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d",
|
||||
c.baseURL,
|
||||
url.PathEscape(owner),
|
||||
url.PathEscape(repo),
|
||||
number,
|
||||
reviewID)
|
||||
body, err := c.doGet(ctx, reqURL)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("get review %d: %w", reviewID, err)
|
||||
}
|
||||
var review struct {
|
||||
Body string `json:"body"`
|
||||
User struct {
|
||||
Login string `json:"login"`
|
||||
} `json:"user"`
|
||||
}
|
||||
if err := json.Unmarshal(body, &review); err != nil {
|
||||
return 0, fmt.Errorf("parse review %d: %w", reviewID, err)
|
||||
}
|
||||
if review.Body == "" {
|
||||
return 0, fmt.Errorf("review %d has empty body", reviewID)
|
||||
}
|
||||
|
||||
// Use a prefix for matching (handles minor trailing whitespace differences)
|
||||
matchPrefix := review.Body
|
||||
if len(matchPrefix) > 200 {
|
||||
matchPrefix = matchPrefix[:200]
|
||||
}
|
||||
|
||||
const pageSize = 50
|
||||
for page := 1; ; page++ {
|
||||
timelineURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/%d/timeline?limit=%d&page=%d",
|
||||
c.baseURL,
|
||||
url.PathEscape(owner),
|
||||
url.PathEscape(repo),
|
||||
number,
|
||||
pageSize,
|
||||
page)
|
||||
tlBody, err := c.doGet(ctx, timelineURL)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("get timeline (page %d): %w", page, err)
|
||||
}
|
||||
var events []TimelineEvent
|
||||
if err := json.Unmarshal(tlBody, &events); err != nil {
|
||||
return 0, fmt.Errorf("parse timeline (page %d): %w", page, err)
|
||||
}
|
||||
for _, ev := range events {
|
||||
if ev.Type == "review" && ev.User.Login == review.User.Login && strings.HasPrefix(ev.Body, matchPrefix) {
|
||||
return ev.ID, nil
|
||||
}
|
||||
}
|
||||
if len(events) < pageSize {
|
||||
break
|
||||
}
|
||||
}
|
||||
return 0, fmt.Errorf("no timeline event found for review %d", reviewID)
|
||||
}
|
||||
|
||||
// EditComment updates the body of an issue/review comment.
|
||||
func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID int64, newBody string) error {
|
||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/comments/%d",
|
||||
@@ -459,3 +523,113 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// GetAuthenticatedUser returns the login of the user authenticated by the token.
|
||||
func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) {
|
||||
reqURL := fmt.Sprintf("%s/api/v1/user", c.baseURL)
|
||||
body, err := c.doGet(ctx, reqURL)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("get authenticated user: %w", err)
|
||||
}
|
||||
var result struct {
|
||||
Login string `json:"login"`
|
||||
}
|
||||
if err := json.Unmarshal(body, &result); err != nil {
|
||||
return "", fmt.Errorf("parse user response: %w", err)
|
||||
}
|
||||
return result.Login, nil
|
||||
}
|
||||
|
||||
// RequestReviewer adds the given user as a requested reviewer on a pull request.
|
||||
// This is idempotent — requesting an already-requested reviewer is a no-op.
|
||||
func (c *Client) RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error {
|
||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/requested_reviewers",
|
||||
c.baseURL,
|
||||
url.PathEscape(owner),
|
||||
url.PathEscape(repo),
|
||||
number)
|
||||
|
||||
payload := struct {
|
||||
Reviewers []string `json:"reviewers"`
|
||||
}{Reviewers: []string{reviewer}}
|
||||
data, err := json.Marshal(payload)
|
||||
if err != nil {
|
||||
return fmt.Errorf("marshal reviewer request: %w", err)
|
||||
}
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data))
|
||||
if err != nil {
|
||||
return fmt.Errorf("create reviewer request: %w", err)
|
||||
}
|
||||
req.Header.Set("Authorization", "token "+c.token)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
|
||||
resp, err := c.http.Do(req)
|
||||
if err != nil {
|
||||
return fmt.Errorf("request reviewer: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
|
||||
body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
|
||||
return fmt.Errorf("request reviewer failed (status %d): %s", resp.StatusCode, body)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ListReviewComments returns the inline comments attached to a specific review.
|
||||
// Paginates through all pages.
|
||||
func (c *Client) ListReviewComments(ctx context.Context, owner, repo string, prNumber int, reviewID int64) ([]ReviewComment, error) {
|
||||
const pageSize = 50
|
||||
var all []ReviewComment
|
||||
for page := 1; ; page++ {
|
||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments?limit=%d&page=%d",
|
||||
c.baseURL,
|
||||
url.PathEscape(owner),
|
||||
url.PathEscape(repo),
|
||||
prNumber,
|
||||
reviewID,
|
||||
pageSize,
|
||||
page)
|
||||
body, err := c.doGet(ctx, reqURL)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list review comments (page %d): %w", page, err)
|
||||
}
|
||||
var batch []ReviewComment
|
||||
if err := json.Unmarshal(body, &batch); err != nil {
|
||||
return nil, fmt.Errorf("parse review comments (page %d): %w", page, err)
|
||||
}
|
||||
all = append(all, batch...)
|
||||
if len(batch) < pageSize {
|
||||
break
|
||||
}
|
||||
}
|
||||
return all, nil
|
||||
}
|
||||
|
||||
// ResolveComment marks an inline review comment as resolved.
|
||||
func (c *Client) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error {
|
||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/comments/%d/resolve",
|
||||
c.baseURL,
|
||||
url.PathEscape(owner),
|
||||
url.PathEscape(repo),
|
||||
commentID)
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, nil)
|
||||
if err != nil {
|
||||
return fmt.Errorf("create resolve request: %w", err)
|
||||
}
|
||||
req.Header.Set("Authorization", "token "+c.token)
|
||||
|
||||
resp, err := c.http.Do(req)
|
||||
if err != nil {
|
||||
return fmt.Errorf("resolve comment: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
|
||||
body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
|
||||
return fmt.Errorf("resolve comment failed (status %d): %s", resp.StatusCode, body)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -5,8 +5,10 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -602,3 +604,142 @@ func TestIsNotFound(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAuthenticatedUser(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/api/v1/user" {
|
||||
t.Errorf("unexpected path: %s", r.URL.Path)
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
if r.Header.Get("Authorization") != "token test-token" {
|
||||
t.Error("missing or wrong auth header")
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprint(w, `{"login":"my-bot","id":42}`)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
login, err := client.GetAuthenticatedUser(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("GetAuthenticatedUser() error = %v", err)
|
||||
}
|
||||
if login != "my-bot" {
|
||||
t.Errorf("login = %q, want %q", login, "my-bot")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequestReviewer(t *testing.T) {
|
||||
var gotBody []byte
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodPost {
|
||||
t.Errorf("expected POST, got %s", r.Method)
|
||||
}
|
||||
expected := "/api/v1/repos/owner/repo/pulls/7/requested_reviewers"
|
||||
if r.URL.Path != expected {
|
||||
t.Errorf("path = %q, want %q", r.URL.Path, expected)
|
||||
}
|
||||
gotBody, _ = io.ReadAll(r.Body)
|
||||
w.WriteHeader(http.StatusCreated)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
err := client.RequestReviewer(context.Background(), "owner", "repo", 7, "bot-user")
|
||||
if err != nil {
|
||||
t.Fatalf("RequestReviewer() error = %v", err)
|
||||
}
|
||||
if !strings.Contains(string(gotBody), `"bot-user"`) {
|
||||
t.Errorf("body = %s, want to contain bot-user", gotBody)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequestReviewer_204(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
|
||||
if err != nil {
|
||||
t.Fatalf("RequestReviewer() should accept 204, got error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequestReviewer_Error(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
fmt.Fprint(w, "no permission")
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for 403 response")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "403") {
|
||||
t.Errorf("error should mention status code: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListReviewComments(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if !strings.Contains(r.URL.Path, "/pulls/1/reviews/42/comments") {
|
||||
t.Errorf("unexpected path: %s", r.URL.Path)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprint(w, `[{"id":100,"path":"main.go","new_position":5,"body":"finding"},{"id":101,"path":"lib.go","new_position":10,"body":"another"}]`)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
comments, err := client.ListReviewComments(context.Background(), "owner", "repo", 1, 42)
|
||||
if err != nil {
|
||||
t.Fatalf("ListReviewComments() error = %v", err)
|
||||
}
|
||||
if len(comments) != 2 {
|
||||
t.Fatalf("got %d comments, want 2", len(comments))
|
||||
}
|
||||
if comments[0].ID != 100 {
|
||||
t.Errorf("comments[0].ID = %d, want 100", comments[0].ID)
|
||||
}
|
||||
if comments[1].Path != "lib.go" {
|
||||
t.Errorf("comments[1].Path = %q, want %q", comments[1].Path, "lib.go")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveComment(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodPost {
|
||||
t.Errorf("expected POST, got %s", r.Method)
|
||||
}
|
||||
if !strings.Contains(r.URL.Path, "/pulls/comments/99/resolve") {
|
||||
t.Errorf("unexpected path: %s", r.URL.Path)
|
||||
}
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
err := client.ResolveComment(context.Background(), "owner", "repo", 99)
|
||||
if err != nil {
|
||||
t.Fatalf("ResolveComment() error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveComment_Error(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
fmt.Fprint(w, "not found")
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
err := client.ResolveComment(context.Background(), "owner", "repo", 99)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for 404 response")
|
||||
}
|
||||
}
|
||||
|
||||
+233
-1
@@ -29,7 +29,12 @@ func ParseResponse(response string) (*ReviewResult, error) {
|
||||
|
||||
var result ReviewResult
|
||||
if err := json.Unmarshal([]byte(cleaned), &result); err != nil {
|
||||
return nil, fmt.Errorf("parse LLM response as JSON: %w\nRaw response: %s", err, response)
|
||||
// LLMs sometimes produce JSON with unescaped quotes inside string values.
|
||||
// Try to repair before giving up.
|
||||
repaired := repairJSON(cleaned)
|
||||
if err2 := json.Unmarshal([]byte(repaired), &result); err2 != nil {
|
||||
return nil, fmt.Errorf("parse LLM response as JSON: %w\nRaw response: %s", err, response)
|
||||
}
|
||||
}
|
||||
|
||||
// Validate verdict
|
||||
@@ -74,3 +79,230 @@ func extractJSON(s string) string {
|
||||
s = strings.TrimSpace(s)
|
||||
return s
|
||||
}
|
||||
|
||||
// repairJSON attempts to fix common LLM JSON issues:
|
||||
// - Unescaped double quotes inside string values
|
||||
//
|
||||
// Strategy: walk the JSON structurally. Object keys are parsed normally (LLMs
|
||||
// get those right). For string VALUES, we find all candidate closing quotes and
|
||||
// pick the LAST one that leaves valid JSON structure afterward — maximizing
|
||||
// string content, which is the correct bias for the "LLM put unescaped quotes
|
||||
// in a string value" failure mode.
|
||||
func repairJSON(s string) string {
|
||||
runes := []rune(s)
|
||||
var out strings.Builder
|
||||
out.Grow(len(s) + 64)
|
||||
|
||||
i := 0
|
||||
for i < len(runes) {
|
||||
c := runes[i]
|
||||
|
||||
if c != '"' {
|
||||
out.WriteRune(c)
|
||||
i++
|
||||
continue
|
||||
}
|
||||
|
||||
// We hit an opening quote. Determine if this is a key or a value.
|
||||
// Keys: the standard JSON parser in LLMs gets keys right, so we parse
|
||||
// them normally (first unescaped quote closes).
|
||||
// Values: may contain unescaped quotes — use the repair heuristic.
|
||||
isValue := isValuePosition(runes, i)
|
||||
|
||||
if !isValue {
|
||||
// Parse key/simple string normally
|
||||
out.WriteRune('"')
|
||||
i++
|
||||
for i < len(runes) {
|
||||
ch := runes[i]
|
||||
if ch == '\\' && i+1 < len(runes) {
|
||||
out.WriteRune(ch)
|
||||
i++
|
||||
out.WriteRune(runes[i])
|
||||
i++
|
||||
continue
|
||||
}
|
||||
if ch == '"' {
|
||||
out.WriteRune('"')
|
||||
i++
|
||||
break
|
||||
}
|
||||
out.WriteRune(ch)
|
||||
i++
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Value string — find the correct close using last-valid-candidate heuristic
|
||||
out.WriteRune('"')
|
||||
i++
|
||||
|
||||
closeIdx := findClosingQuote(runes, i)
|
||||
|
||||
// Write everything between open and close, escaping interior quotes
|
||||
for j := i; j < closeIdx; j++ {
|
||||
ch := runes[j]
|
||||
if ch == '\\' && j+1 < closeIdx {
|
||||
// Already-escaped sequence — pass through
|
||||
out.WriteRune(ch)
|
||||
j++
|
||||
out.WriteRune(runes[j])
|
||||
} else if ch == '"' {
|
||||
out.WriteRune('\\')
|
||||
out.WriteRune('"')
|
||||
} else {
|
||||
out.WriteRune(ch)
|
||||
}
|
||||
}
|
||||
|
||||
// Write the closing quote
|
||||
out.WriteRune('"')
|
||||
i = closeIdx + 1
|
||||
}
|
||||
|
||||
return out.String()
|
||||
}
|
||||
|
||||
// isValuePosition determines if the quote at position i is opening a JSON value
|
||||
// string (as opposed to an object key). We only apply repair to values that
|
||||
// follow ':' since those are the free-text fields where LLMs produce unescaped
|
||||
// quotes. Array elements and keys are left alone (parsed normally).
|
||||
func isValuePosition(runes []rune, i int) bool {
|
||||
// Look backward, skipping whitespace, for the preceding structural char
|
||||
j := i - 1
|
||||
for j >= 0 && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
|
||||
j--
|
||||
}
|
||||
if j < 0 {
|
||||
return false
|
||||
}
|
||||
// After ':' → definitely a value
|
||||
return runes[j] == ':'
|
||||
}
|
||||
|
||||
// findClosingQuote finds the index of the true closing quote for a JSON string
|
||||
// value starting at position start (the character after the opening quote).
|
||||
// It collects all unescaped quote candidates and returns the FIRST one that
|
||||
// produces valid JSON continuation (deeper lookahead verifies the next token).
|
||||
func findClosingQuote(runes []rune, start int) int {
|
||||
// Collect all candidate positions for the closing quote.
|
||||
var candidates []int
|
||||
for j := start; j < len(runes); j++ {
|
||||
if runes[j] == '\\' {
|
||||
j++ // skip escaped character
|
||||
continue
|
||||
}
|
||||
if runes[j] == '"' {
|
||||
candidates = append(candidates, j)
|
||||
}
|
||||
}
|
||||
|
||||
if len(candidates) == 0 {
|
||||
return len(runes)
|
||||
}
|
||||
|
||||
if len(candidates) == 1 {
|
||||
return candidates[0]
|
||||
}
|
||||
|
||||
// Try candidates from FIRST to LAST. The correct closing quote is the
|
||||
// earliest one that produces valid JSON structure after it (verified by
|
||||
// deeper lookahead that checks the next token is a valid JSON start).
|
||||
for _, idx := range candidates {
|
||||
if isValidJSONAfterClose(runes, idx+1) {
|
||||
return idx
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback: return the last candidate
|
||||
return candidates[len(candidates)-1]
|
||||
}
|
||||
|
||||
// isValidJSONAfterClose checks whether the runes after a candidate closing quote
|
||||
// look like valid JSON continuation for a VALUE string. Since we only use this
|
||||
// for value positions, ':' is NOT a valid continuation (values are never keys).
|
||||
// Checks deeper structure to avoid being fooled by JSON-like content in strings.
|
||||
func isValidJSONAfterClose(runes []rune, pos int) bool {
|
||||
j := pos
|
||||
for j < len(runes) && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
|
||||
j++
|
||||
}
|
||||
|
||||
if j >= len(runes) {
|
||||
return true
|
||||
}
|
||||
|
||||
next := runes[j]
|
||||
if next == '}' || next == ']' {
|
||||
// Closing a container. Verify what follows the close is also valid:
|
||||
// another structural char, comma, or EOF.
|
||||
return isValidAfterContainerClose(runes, j+1)
|
||||
}
|
||||
if next == ',' {
|
||||
// After comma, must be followed by a valid JSON token
|
||||
j++
|
||||
for j < len(runes) && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
|
||||
j++
|
||||
}
|
||||
if j >= len(runes) {
|
||||
return false // trailing comma with nothing after — invalid
|
||||
}
|
||||
return isJSONTokenStart(runes, j)
|
||||
}
|
||||
// ':' is NOT valid here — we're in a value position, not a key.
|
||||
// Any other character is also invalid.
|
||||
return false
|
||||
}
|
||||
|
||||
// isValidAfterContainerClose checks that after a } or ], the continuation is
|
||||
// structurally valid: more closes, comma+token, or EOF.
|
||||
func isValidAfterContainerClose(runes []rune, pos int) bool {
|
||||
j := pos
|
||||
for j < len(runes) && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
|
||||
j++
|
||||
}
|
||||
if j >= len(runes) {
|
||||
return true
|
||||
}
|
||||
next := runes[j]
|
||||
if next == '}' || next == ']' {
|
||||
return isValidAfterContainerClose(runes, j+1)
|
||||
}
|
||||
if next == ',' {
|
||||
j++
|
||||
for j < len(runes) && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
|
||||
j++
|
||||
}
|
||||
if j >= len(runes) {
|
||||
return false
|
||||
}
|
||||
return isJSONTokenStart(runes, j)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// isJSONTokenStart returns true if the rune could begin a JSON value or key.
|
||||
// For keywords (true/false/null), verifies the full keyword is present.
|
||||
func isJSONTokenStart(runes []rune, pos int) bool {
|
||||
if pos >= len(runes) {
|
||||
return false
|
||||
}
|
||||
r := runes[pos]
|
||||
switch {
|
||||
case r == '"': // string
|
||||
return true
|
||||
case r == '{' || r == '[': // object or array
|
||||
return true
|
||||
case r == 't': // true
|
||||
return pos+4 <= len(runes) && string(runes[pos:pos+4]) == "true"
|
||||
case r == 'f': // false
|
||||
return pos+5 <= len(runes) && string(runes[pos:pos+5]) == "false"
|
||||
case r == 'n': // null
|
||||
return pos+4 <= len(runes) && string(runes[pos:pos+4]) == "null"
|
||||
case r >= '0' && r <= '9': // number
|
||||
return true
|
||||
case r == '-': // negative number
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package review
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -112,3 +113,112 @@ func TestParseResponse_MarkdownFencesNoLang(t *testing.T) {
|
||||
t.Errorf("expected APPROVE, got %q", result.Verdict)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseResponse_UnescapedQuotesInStrings(t *testing.T) {
|
||||
// Real failure from CI: Sonnet puts unescaped quotes like (e.g. "28") in findings
|
||||
input := `{"verdict": "APPROVE", "summary": "Clean PR", "findings": [{"severity": "NIT", "file": "ci/Dockerfile", "line": 14, "finding": "The comment says OTP_VERSION is the major version (e.g. \"28\") but it actually contains unescaped quotes like (e.g. "28") which breaks JSON"}], "recommendation": "Ship it"}`
|
||||
|
||||
result, err := ParseResponse(input)
|
||||
if err != nil {
|
||||
t.Fatalf("expected repair to handle unescaped quotes, got error: %v", err)
|
||||
}
|
||||
if result.Verdict != "APPROVE" {
|
||||
t.Errorf("expected APPROVE, got %q", result.Verdict)
|
||||
}
|
||||
if len(result.Findings) != 1 {
|
||||
t.Fatalf("expected 1 finding, got %d", len(result.Findings))
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepairJSON_NoOpOnValid(t *testing.T) {
|
||||
valid := `{"key": "value", "num": 42}`
|
||||
result := repairJSON(valid)
|
||||
if result != valid {
|
||||
t.Errorf("repairJSON should not modify valid JSON\n got: %s\n want: %s", result, valid)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepairJSON_FixesUnescapedQuotes(t *testing.T) {
|
||||
// Interior quote followed by non-structural character
|
||||
input := `{"msg": "use "foo" here"}`
|
||||
result := repairJSON(input)
|
||||
|
||||
// Should be parseable now
|
||||
var m map[string]interface{}
|
||||
if err := json.Unmarshal([]byte(result), &m); err != nil {
|
||||
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepairJSON_InteriorQuoteBeforeComma(t *testing.T) {
|
||||
// Bug reported by reviewer: interior quoted word immediately before a comma
|
||||
input := `{"msg": "say "yes", and go"}`
|
||||
result := repairJSON(input)
|
||||
|
||||
var m map[string]interface{}
|
||||
if err := json.Unmarshal([]byte(result), &m); err != nil {
|
||||
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
|
||||
}
|
||||
// The full string content should be preserved
|
||||
msg, ok := m["msg"].(string)
|
||||
if !ok {
|
||||
t.Fatal("msg field missing or not a string")
|
||||
}
|
||||
if msg != `say "yes", and go` {
|
||||
t.Errorf("unexpected msg content: %q", msg)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepairJSON_InteriorQuoteBeforeCloseBrace(t *testing.T) {
|
||||
// Bug reported by reviewer: JSON-shaped syntax inside string values
|
||||
input := `{"msg": "input map {"key": "val"} caused error"}`
|
||||
result := repairJSON(input)
|
||||
|
||||
var m map[string]interface{}
|
||||
if err := json.Unmarshal([]byte(result), &m); err != nil {
|
||||
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepairJSON_MultipleFields(t *testing.T) {
|
||||
// Multiple string fields with unescaped quotes in different positions
|
||||
input := `{"a": "hello "world"", "b": "foo"}`
|
||||
result := repairJSON(input)
|
||||
|
||||
var m map[string]interface{}
|
||||
if err := json.Unmarshal([]byte(result), &m); err != nil {
|
||||
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
|
||||
}
|
||||
if _, ok := m["b"]; !ok {
|
||||
t.Error("expected 'b' field to be preserved")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepairJSON_PreservesEscapedQuotes(t *testing.T) {
|
||||
// Already-escaped quotes should not be double-escaped
|
||||
input := `{"msg": "already \"escaped\" here"}`
|
||||
result := repairJSON(input)
|
||||
|
||||
if result != input {
|
||||
t.Errorf("repairJSON should not modify already-escaped quotes\n got: %s\n want: %s", result, input)
|
||||
}
|
||||
|
||||
var m map[string]interface{}
|
||||
if err := json.Unmarshal([]byte(result), &m); err != nil {
|
||||
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepairJSON_ComplexNestedContent(t *testing.T) {
|
||||
// Combines both reviewer bugs: quoted words before commas AND JSON-like content
|
||||
input := `{"verdict": "APPROVE", "findings": [{"finding": "The map {"key": "val"} and (e.g. "28") and say "yes", then stop"}]}`
|
||||
result := repairJSON(input)
|
||||
|
||||
var parsed map[string]interface{}
|
||||
if err := json.Unmarshal([]byte(result), &parsed); err != nil {
|
||||
t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
|
||||
}
|
||||
if parsed["verdict"] != "APPROVE" {
|
||||
t.Errorf("expected verdict APPROVE, got %v", parsed["verdict"])
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user