Compare commits

...

19 Commits

Author SHA1 Message Date
Rodin 489457c184 ci: retrigger after LLM_BASE_URL secret fix
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 20s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m27s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m52s
2026-05-04 23:15:08 -07:00
Rodin 25d1a670bf fix: redesign repairJSON to handle all reviewer-reported edge cases
CI / test (pull_request) Successful in 11s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Failing after 10s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Failing after 10s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 11s
Rewrites the JSON repair algorithm to address two correctness bugs
identified in code review:

1. Interior quoted word before comma: "say "yes", and go" was
   misidentified as structural close because "," followed the quote.

2. JSON-shaped content in strings: {"key": "val"} inside a string
   value was being parsed as actual JSON structure.

The new approach:
- Distinguishes keys from values (only values need repair)
- Uses first-valid-candidate scan with deep lookahead
- Verifies that after a candidate close, the continuation is not just
  a structural char but a complete valid JSON pattern
- Validates keyword tokens (true/false/null) fully, not just first char
- Checks container closes recursively for valid continuation

Adds comprehensive tests for all reported edge cases plus a complex
combined scenario with nested JSON-like content, quoted words before
commas, and multiple failure modes in one string.
2026-05-04 21:27:39 -07:00
Rodin 80a9a7675b fix: repair unescaped quotes in LLM JSON responses
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 12s
LLMs (especially Sonnet) sometimes emit JSON with unescaped double
quotes inside string values, e.g. (e.g. "28") instead of properly
escaping them. This caused parse failures in CI.

Add a repairJSON fallback that uses a character-by-character scanner
to identify interior quotes (those not followed by structural JSON
characters) and escape them before retrying the parse.

Fixes sonnet-review failures on gargoyle PR #551.
2026-05-03 09:47:22 -07:00
rodin 8d8a249481 Merge pull request 'fix: supersede ALL old reviews, not just the most recent' (#43) from fix/supersede-all-old-reviews into main
CI / test (push) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
Release / release (push) Successful in 31s
2026-05-02 20:35:23 +00:00
Rodin a0fd882b0d fix: address review findings
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 24s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 37s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m4s
- Tighten timeline matching: also check ev.User.Login matches
  the review author (prevents collision on identical body prefix)
- Remove unused sharedTokenMode variable (inline condition)
- Aggregate resolution failures with warn-level summary
2026-05-02 13:31:59 -07:00
Rodin d4bf13eeab fix: supersede ALL old reviews, not just the most recent
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 22s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 46s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m7s
Previously findOwnReview returned only the single most-recent matching
review, so on PRs with multiple force-pushes only the latest old review
got superseded. The rest accumulated as unsuperseded stale reviews.

Changes:
- Add findAllOwnReviews() to collect all non-superseded matching reviews
- Loop over all old reviews in the supersede phase
- Add GetTimelineReviewCommentIDForReview() to find comment IDs by
  review ID (fetches review body, matches in timeline by prefix)
- Each old review gets independently superseded and its inline comments
  resolved

The old findOwnReview is kept for backward compat (tested, may be
useful as a utility).
2026-05-02 13:28:03 -07:00
rodin 23443ef378 Merge pull request 'feat: resolve old inline comments when superseding review' (#42) from feat/27-resolve-inline-comments into main
CI / test (push) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
Release / release (push) Successful in 32s
2026-05-02 19:18:41 +00:00
Rodin bc5a4a1dcd feat: resolve old inline comments when superseding review
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 22s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 44s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 46s
Closes #27

After superseding an old review, resolves all its inline comments via
POST /pulls/comments/{id}/resolve. This clears unresolved conversation
markers from the PR timeline and diff view.

New API methods:
- ListReviewComments: paginated GET /repos/.../pulls/{n}/reviews/{id}/comments
- ResolveComment: POST /repos/.../pulls/comments/{id}/resolve

Behavior:
- Only resolves after successful supersede (gated on supersedeOK)
- Aggregates failures and logs at warn level
- Truncates error bodies to 256 bytes (security)
- Non-fatal: review still posts even if resolution fails
2026-05-02 12:15:52 -07:00
rodin d30f3d4278 Merge pull request 'feat: self-request as reviewer before posting' (#41) from feat/35-self-request-reviewer into main
CI / test (push) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
2026-05-02 19:11:15 +00:00
Rodin 2507ee22e7 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
- 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
2026-05-02 12:09:25 -07:00
Rodin c39845ca03 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
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.
2026-05-02 12:04:55 -07:00
rodin cd601bdcf4 Merge pull request 'fix: trim trailing slash from giteaURL when building review link' (#40) from fix/url-normalization into main
CI / test (push) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
2026-05-02 18:52:13 +00:00
Rodin 50091941e1 fix: trim trailing slash from giteaURL when building review link
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 20s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 28s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 41s
Prevents double-slash in supersede URL if GITEA_URL ends with '/'.
Aligns with how gitea.NewClient already normalizes the base URL.
2026-05-02 11:49:24 -07:00
rodin ed06cdd942 Merge pull request 'fix: post new review first, then supersede old with link' (#39) from fix/34-supersede-order into main
CI / test (push) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
2026-05-02 18:46:50 +00:00
Rodin ed69d26e87 fix: post new review first, then supersede old with link
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 22s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 44s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m8s
Changes the order of operations:
1. POST new review (gets non-stale badge immediately)
2. PATCH old review with superseded message linking to the new one

This gives the superseded comment a clickable link to the current
review, making navigation between review iterations easy.

buildSupersededBody now accepts a newReviewURL parameter.
2026-05-02 11:43:53 -07:00
rodin da586a512a Merge pull request 'feat: always post fresh review, supersede old with collapsed body' (#38) from feat/34-always-post-fresh into main
CI / test (push) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
2026-05-02 18:36:42 +00:00
Rodin f6baa41b2c fix: remove findOwnReviewStrict, use findOwnReview directly
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 42s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m31s
The strict authorship check compared reviewer-name to User.Login which
could mismatch. The sentinel is already role-specific (e.g.
<!-- review-bot:sonnet -->) and Gitea's API blocks editing others'
comments (403). Defense-in-depth via login comparison is unnecessary
complexity that introduced a bug. Removed.
2026-05-02 11:33:57 -07:00
Rodin ecbae332f4 fix: address review findings
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m17s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m22s
- findOwnReview: skip superseded reviews, pick highest ID (most recent)
- findOwnReviewStrict: verify authorship before superseding (defense-in-depth)
- buildSupersededBody: handle empty commitSHA gracefully
- Tests: add cases for superseded skip, highest-ID selection
2026-05-02 11:30:34 -07:00
Rodin fdd75699d9 feat: always post fresh review, supersede old with collapsed body
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m2s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m25s
Closes #34

- Remove reviewUnchanged() skip logic — every push gets a fresh review
- Remove edit-in-place (PATCH same body) — always POST new
- Supersede old review: PATCH with struck-through banner + collapsed
  original body in <details> for historical reference
- Add commit footer to every review: 'Evaluated against <sha>'
- Remove --update-existing flag (no longer needed)
- Add CommitID field to Review struct
- Add TestBuildSupersededBody tests
2026-05-02 11:26:06 -07:00
6 changed files with 877 additions and 147 deletions
+122 -66
View File
@@ -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
View File
@@ -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
View File
@@ -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
}
+141
View File
@@ -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
View File
@@ -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
}
+110
View File
@@ -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"])
}
}