diff --git a/.github/workflows/osv-scanner.yml b/.github/workflows/osv-scanner.yml index a2215e95..3c7938da 100644 --- a/.github/workflows/osv-scanner.yml +++ b/.github/workflows/osv-scanner.yml @@ -23,7 +23,11 @@ jobs: - uses: actions/setup-go@v5 with: go-version-file: go.mod - - run: go install github.com/google/osv-scanner/v2/cmd/osv-scanner@latest + - name: Install osv-scanner + run: | + curl -fsSL https://github.com/google/osv-scanner/releases/latest/download/osv-scanner_linux_amd64 -o /usr/local/bin/osv-scanner + chmod +x /usr/local/bin/osv-scanner + osv-scanner --version - run: mkdir -p security_issues - run: osv-scanner scan source --recursive --format json --config osv-scanner.toml --no-call-analysis=go --experimental-exclude=debug --experimental-exclude=scripts --experimental-exclude=tests --experimental-exclude=.livereview_pgdata --experimental-exclude=.lrdata --experimental-exclude=livereview_pgdata --experimental-exclude=lrdata . > security_issues/osv-scanner-ci.json - uses: actions/upload-artifact@v4 diff --git a/docs/integrations/gitea/gitea_issues.md b/docs/integrations/gitea/gitea_issues.md new file mode 100644 index 00000000..34da91d4 --- /dev/null +++ b/docs/integrations/gitea/gitea_issues.md @@ -0,0 +1,49 @@ +# Gitea Issues + +This doc tracks Gitea integration issues and their implemented solutions. + +## Resolved Issues + +### 1. Missing Severity Level in Initial Comments +**Issue:** +When the AI posted code reviews on a Gitea PR, the severity level (e.g., `**Severity: critical**`) was missing from the comment block. +**Solution:** +The Gitea provider previously posted raw comment content. This was fixed by introducing `formatGiteaComment` in [internal/providers/gitea/gitea_provider.go#L274](internal/providers/gitea/gitea_provider.go#L274). This function standardizes the format to match the GitHub/GitLab providers, ensuring that severity and suggestions are properly injected and sanitized before the comment is posted. + +### 2. General Replies Displacing Inline Threading +**Issue:** +When a user replied to an inline review comment on Gitea, the bot's subsequent reply broke out of the inline discussion and was posted at the very bottom of the PR timeline as a quoted general comment. +**Solution:** +Gitea webhooks for replies often omit the exact `position` line data while retaining the `review_id`. The reply routing logic in [internal/provider_output/gitea/api_client.go#L59](internal/provider_output/gitea/api_client.go#L59) was updated to aggressively trigger metadata enrichment whenever an inline reply lacks `position`. The [enrichCommentMetadata](internal/provider_output/gitea/api_client.go#L220) function now dynamically fetches the parent comment's line coordinates to ensure the bot responds properly within the inline thread rather than falling back to a general quote block. + +### 3. Inline Comments Disregarded as PR Requests +**Issue:** +Whenever a user tried to comment inline on the code, Gitea would send a webhook that the system incorrectly disregarded as a general PR request, completely ignoring the comment content. +**Solution:** +This occurred because Gitea assigns the `reviewed` action to `pull_request_review_comment` webhooks when an inline comment is created. The webhook parser ([internal/provider_input/gitea/gitea_conversion.go#L83](internal/provider_input/gitea/gitea_conversion.go#L83)) was strictly filtering out any action other than `created`. The logic was updated to explicitly process `reviewed` actions and intelligently fall back to the `Review` object if the raw `Comment` body was omitted in the payload. The unified processor also handles this special case in [internal/api/unified_processor_v2.go#L88](internal/api/unified_processor_v2.go#L88). +## TODO + +### 4. Race Condition Handling for Multiple Bot Comments +**Issue:** +When livereview posts multiple comments in quick succession in response to a single user comment, the enrichment logic may select an earlier, incomplete comment instead of the most recent and comprehensive one. This can lead to processing outdated or less detailed responses. + +**Current Status:** +- ✅ **Implemented:** Enhanced enrichment logic to collect all suitable comments and prioritize by timestamp +- ✅ **Fixed:** Removed early break after finding first suitable comment +- ✅ **Added:** Content comparison to handle duplicate comment bodies +- 🔄 **In Progress:** Testing and monitoring for race condition scenarios + +**Solution Details:** +1. **Collect all suitable comments** instead of breaking after first match +2. **Prioritize by latest timestamp** using `UpdatedAt` field comparison +3. **Skip duplicate content** using `seenContents` map to prevent race processing +4. **Select most comprehensive** response when multiple comments contain bot mention + +**Files Modified:** +- `internal/provider_input/gitea/gitea_provider.go` - Updated enrichment logic in `FetchMergeRequestData` +- `internal/provider_input/gitea/gitea_types.go` - Added `DiffHunk` field to `GiteaReviewComment` struct + +**Next Steps:** +- Monitor webhook processing logs for race condition scenarios +- Verify that latest/most comprehensive comment is consistently selected +- Consider additional heuristics if timestamp-based selection proves insufficient \ No newline at end of file diff --git a/internal/api/unified_processor_v2.go b/internal/api/unified_processor_v2.go index 48f42993..47413f45 100644 --- a/internal/api/unified_processor_v2.go +++ b/internal/api/unified_processor_v2.go @@ -25,6 +25,8 @@ import ( gitlabmentions "github.com/livereview/internal/providers/gitlab" gl "github.com/livereview/internal/providers/gitlab" "github.com/livereview/internal/reviewmodel" + networkbitbucket "github.com/livereview/network/providers/bitbucket" + networkgithub "github.com/livereview/network/providers/github" networkgitea "github.com/livereview/network/providers/gitea" ) @@ -85,6 +87,16 @@ func (p *UnifiedProcessorV2Impl) CheckResponseWarrant(event UnifiedWebhookEventV commentBody := strings.TrimSpace(event.Comment.Body) if commentBody == "" { + // Gitea Special Case: 'reviewed' action often has empty body but carries inline comments + if event.Provider == "gitea" && event.Comment.Metadata != nil && event.Comment.Metadata["action"] == "reviewed" { + log.Printf("[DEBUG] Empty body for Gitea 'reviewed' action; allowing warrant for review scan") + return true, ResponseScenarioV2{ + Type: "review_submission", + Reason: "Gitea review submission (requires scan of inline comments)", + Confidence: 0.5, // Low confidence until we find a mention in the scan + Metadata: map[string]interface{}{"action": "reviewed"}, + } + } return hardFailure("comment body empty; cannot evaluate warrant", "event.comment.body") } @@ -714,7 +726,7 @@ func (p *UnifiedProcessorV2Impl) checkGitHubParentCommentAuthor(event UnifiedWeb apiURL = fmt.Sprintf("https://api.github.com/repos/%s/issues/comments/%s", repoFullName, parentID) } - req, err := http.NewRequest("GET", apiURL, nil) + req, err := networkgithub.NewRequest(http.MethodGet, apiURL, nil) if err != nil { return false, err } @@ -723,8 +735,8 @@ func (p *UnifiedProcessorV2Impl) checkGitHubParentCommentAuthor(event UnifiedWeb req.Header.Set("Accept", "application/vnd.github.v3+json") req.Header.Set("User-Agent", "LiveReview-Bot") - client := &http.Client{Timeout: 30 * time.Second} - resp, err := client.Do(req) + client := networkgithub.NewHTTPClient(30 * time.Second) + resp, err := networkgithub.Do(client, req) if err != nil { return false, err } @@ -811,7 +823,7 @@ func (p *UnifiedProcessorV2Impl) checkBitbucketParentCommentAuthor(event Unified } apiURL := fmt.Sprintf("https://api.bitbucket.org/2.0/repositories/%s/%s/pullrequests/%s/comments/%s", workspace, repository, prNumber, parentID) - req, err := http.NewRequest("GET", apiURL, nil) + req, err := networkbitbucket.NewRequestWithContext(context.Background(), http.MethodGet, apiURL, nil) if err != nil { return false, err } @@ -819,8 +831,8 @@ func (p *UnifiedProcessorV2Impl) checkBitbucketParentCommentAuthor(event Unified req.SetBasicAuth(email, token.PatToken) req.Header.Set("Accept", "application/json") - client := &http.Client{Timeout: 10 * time.Second} - resp, err := client.Do(req) + client := networkbitbucket.NewHTTPClient(10 * time.Second) + resp, err := networkbitbucket.Do(client, req) if err != nil { return false, err } @@ -1586,25 +1598,28 @@ func (p *UnifiedProcessorV2Impl) buildGiteaArtifactFromEvent(ctx context.Context log.Printf("[DEBUG] Building Gitea artifact for repo=%s org_id=%d", event.Repository.FullName, orgID) patchURL := "" - if event.MergeRequest.Metadata != nil { - if rawPatchURL, ok := event.MergeRequest.Metadata["patch_url"].(string); ok { - patchURL = strings.TrimSpace(rawPatchURL) - } + // Construct the official API patch URL which reliably accepts PAT authentication. + // We prioritize this over the UI-based patch_url often found in webhook metadata. + baseURL := strings.TrimRight(token.ProviderURL, "/") + repoFullName := event.Repository.FullName + prNumber := event.MergeRequest.Number + + if baseURL != "" && repoFullName != "" && prNumber > 0 { + patchURL = fmt.Sprintf("%s/api/v1/repos/%s/pulls/%d.patch", + baseURL, repoFullName, prNumber) } - if patchURL == "" { - // Use provider URL and metadata to construct API patch URL reliably. - // We avoid brittle WebURL path manipulation by using known metadata. - baseURL := strings.TrimRight(token.ProviderURL, "/") - repoFullName := event.Repository.FullName - prNumber := event.MergeRequest.Number - if baseURL != "" && repoFullName != "" && prNumber > 0 { - patchURL = fmt.Sprintf("%s/api/v1/repos/%s/pulls/%d.patch", - baseURL, repoFullName, prNumber) - } else if webURL := strings.TrimSpace(event.MergeRequest.WebURL); webURL != "" { - // Fallback: Gitea supports appending .patch to the UI URL. - // This is a safe last-resort if API-specific metadata is incomplete. - patchURL = strings.TrimRight(webURL, "/") + ".patch" + // Fallback to metadata or web URL if API URL construction is not possible + if patchURL == "" { + if event.MergeRequest.Metadata != nil { + if rawPatchURL, ok := event.MergeRequest.Metadata["patch_url"].(string); ok { + patchURL = strings.TrimSpace(rawPatchURL) + } + } + if patchURL == "" { + if webURL := strings.TrimSpace(event.MergeRequest.WebURL); webURL != "" { + patchURL = strings.TrimRight(webURL, "/") + ".patch" + } } } if patchURL == "" { @@ -1612,6 +1627,7 @@ func (p *UnifiedProcessorV2Impl) buildGiteaArtifactFromEvent(ctx context.Context } client := networkgitea.NewHTTPClient(20 * time.Second) + log.Printf("[DEBUG] Fetching Gitea patch from URL: %s", patchURL) patchContent, err := networkgitea.FetchPatchContent(ctx, client, patchURL, token.PatToken) if err != nil { return nil, fmt.Errorf("failed to fetch gitea patch: %w", err) diff --git a/internal/api/webhook_orchestrator_v2.go b/internal/api/webhook_orchestrator_v2.go index d9ee2168..76c666dc 100644 --- a/internal/api/webhook_orchestrator_v2.go +++ b/internal/api/webhook_orchestrator_v2.go @@ -308,8 +308,8 @@ func (wo *WebhookOrchestratorV2) processEventAsync(ctx context.Context, event *U case "discussion_reply": log.Printf("[INFO] Discussion reply scenario - handling as comment reply") wo.handleCommentReplyFlow(processingCtx, event, provider, timeline, orgID) - case "content_trigger": - log.Printf("[INFO] Content trigger scenario - handling as comment reply") + case "content_trigger", "review_submission": + log.Printf("[INFO] %s scenario - handling as comment reply", scenario.Type) wo.handleCommentReplyFlow(processingCtx, event, provider, timeline, orgID) default: log.Printf("[WARN] Unknown response scenario: %s", scenario.Type) @@ -325,6 +325,11 @@ func (wo *WebhookOrchestratorV2) processEventAsync(ctx context.Context, event *U func (wo *WebhookOrchestratorV2) handleCommentReplyFlow(ctx context.Context, event *UnifiedWebhookEventV2, provider WebhookProviderV2, timeline *UnifiedTimelineV2, orgID int64) { log.Printf("[INFO] Processing comment reply flow for event %s/%s", event.EventType, event.Provider) + if event.Comment == nil || strings.TrimSpace(event.Comment.Body) == "" { + log.Printf("[INFO] Skipping comment reply flow: comment body is empty (even after potential enrichment)") + return + } + // Generate AI response response, learning, usage, err := wo.unifiedProcessor.ProcessCommentReply(ctx, *event, timeline, orgID) if err != nil { diff --git a/internal/provider_input/gitea/gitea_conversion.go b/internal/provider_input/gitea/gitea_conversion.go index 8902ee0c..b9bcc6a7 100644 --- a/internal/provider_input/gitea/gitea_conversion.go +++ b/internal/provider_input/gitea/gitea_conversion.go @@ -80,13 +80,26 @@ func ConvertGiteaPullRequestReviewCommentEvent(body []byte) (*UnifiedWebhookEven return nil, fmt.Errorf("failed to parse Gitea PR review comment webhook: %w", err) } - if payload.Action != "created" { + if payload.Action != "created" && payload.Action != "reviewed" { log.Printf("[DEBUG] Ignoring Gitea pull_request_review_comment action: %s", payload.Action) return nil, fmt.Errorf("pull_request_review_comment event ignored (action=%s)", payload.Action) } if payload.Comment == nil { - return nil, fmt.Errorf("comment is nil in payload") + if payload.Action == "reviewed" && payload.Review != nil { + // Fallback to review content if it's a review event and comment is not explicitly provided + payload.Comment = &GiteaV2Comment{ + ID: payload.Review.ID, + HTMLURL: payload.Review.HTMLURL, + User: payload.Review.User, + Body: payload.Review.Content, + CreatedAt: payload.Review.CreatedAt, + UpdatedAt: payload.Review.UpdatedAt, + ReviewID: payload.Review.ID, + } + } else { + return nil, fmt.Errorf("comment is nil in payload") + } } if payload.PullRequest == nil { return nil, fmt.Errorf("pull_request is nil in payload") @@ -108,6 +121,11 @@ func ConvertGiteaPullRequestReviewCommentEvent(body []byte) (*UnifiedWebhookEven Actor: convertGiteaUserToUnified(payload.Sender), } + // Tag the action in metadata so the processor knows how to handle empty summaries + if event.Comment != nil && event.Comment.Metadata != nil { + event.Comment.Metadata["action"] = payload.Action + } + return event, nil } @@ -182,6 +200,13 @@ func convertGiteaCommentToUnified(comment *GiteaV2Comment) *UnifiedCommentV2 { } unified.Metadata["comment_type"] = "review_comment" + } else if comment.ReviewID > 0 || comment.InReplyTo > 0 { + // No path/position, but associated with a review or replying to a review comment. + // This happens when a user clicks "Reply" inside an inline review discussion — + // Gitea fires an issue_comment event, but the reply belongs to the review thread. + unified.Metadata["comment_type"] = "review_reply" + log.Printf("[DEBUG] Gitea comment %d tagged as review_reply: review_id=%d, in_reply_to=%d", + comment.ID, comment.ReviewID, comment.InReplyTo) } else { unified.Metadata["comment_type"] = "issue_comment" } @@ -190,6 +215,7 @@ func convertGiteaCommentToUnified(comment *GiteaV2Comment) *UnifiedCommentV2 { if comment.InReplyTo > 0 { inReplyTo := strconv.FormatInt(comment.InReplyTo, 10) unified.InReplyToID = &inReplyTo + unified.Metadata["in_reply_to"] = comment.InReplyTo } // Review context @@ -287,12 +313,16 @@ func convertGiteaUserToUnified(user *GiteaV2User) UnifiedUserV2 { // Gitea uses "LEFT" (old/base) and "RIGHT" (new/head) // Unified types use "old", "new", "context" func convertGiteaSideToLineType(side string) string { + log.Printf("[DEBUG] convertGiteaSideToLineType: input side='%s'", side) + result := "new" // Default to new side switch strings.ToUpper(side) { case "LEFT": - return "old" + result = "old" case "RIGHT": - return "new" + result = "new" default: - return "new" // Default to new side + result = "new" // Default to new side } + log.Printf("[DEBUG] convertGiteaSideToLineType: result='%s' for input='%s'", result, side) + return result } diff --git a/internal/provider_input/gitea/gitea_provider.go b/internal/provider_input/gitea/gitea_provider.go index ca2e1b6e..4d69af40 100644 --- a/internal/provider_input/gitea/gitea_provider.go +++ b/internal/provider_input/gitea/gitea_provider.go @@ -12,6 +12,8 @@ import ( "log" "net/http" "net/url" + "regexp" + "strconv" "strings" "time" @@ -37,7 +39,6 @@ type GiteaOutputClient interface { PostReviewComments(mr UnifiedMergeRequestV2, token string, comments []UnifiedReviewCommentV2) error } -// GiteaV2Provider implements webhook provider behaviour for Gitea. type GiteaV2Provider struct { db *sql.DB output GiteaOutputClient @@ -97,7 +98,6 @@ func (p *GiteaV2Provider) ConvertCommentEvent(headers map[string]string, body [] switch eventType { case "issue_comment": - log.Printf("[DEBUG] Processing Gitea issue_comment event") event, err = ConvertGiteaIssueCommentEvent(body) case "pull_request_comment", "pull_request_review_comment": log.Printf("[DEBUG] Processing Gitea pull_request_review_comment event") @@ -133,7 +133,7 @@ func (p *GiteaV2Provider) FetchMergeRequestData(event *UnifiedWebhookEventV2) er return fmt.Errorf("no merge request in event") } - _, baseURL, err := FindIntegrationTokenForGiteaRepo(p.db, event.Repository.FullName) + token, baseURL, err := FindIntegrationTokenForGiteaRepo(p.db, event.Repository.FullName) if err != nil { return fmt.Errorf("failed to get Gitea token: %w", err) } @@ -152,8 +152,6 @@ func (p *GiteaV2Provider) FetchMergeRequestData(event *UnifiedWebhookEventV2) er log.Printf("[INFO] Fetching PR data for Gitea PR %s/%s#%d (base_url=%s)", owner, repo, prNumber, baseURL) - // For now, just log and return success - // Full implementation will be added in next iteration if event.MergeRequest.Metadata == nil { event.MergeRequest.Metadata = map[string]interface{}{} } @@ -161,6 +159,95 @@ func (p *GiteaV2Provider) FetchMergeRequestData(event *UnifiedWebhookEventV2) er event.MergeRequest.Metadata["pull_request_number"] = event.MergeRequest.Number event.MergeRequest.Metadata["base_url"] = baseURL + // Enrichment logic for Gitea 'reviewed' events + // These events often have empty bodies but carry inline mentions in the review's comments + if event.Comment != nil && event.Comment.Metadata != nil { + log.Printf("[DEBUG] Gitea enrichment check: action=%v, metadata=%+v", event.Comment.Metadata["action"], event.Comment.Metadata) + if event.Comment.Metadata["action"] == "reviewed" { + reviewIDRaw := event.Comment.Metadata["review_id"] + log.Printf("[DEBUG] Gitea enrichment: found 'reviewed' action, reviewIDRaw=%v (type %T)", reviewIDRaw, reviewIDRaw) + var reviewID int64 + switch v := reviewIDRaw.(type) { + case int64: + reviewID = v + case int: + reviewID = int64(v) + case float64: + reviewID = int64(v) + } + log.Printf("[DEBUG] Gitea enrichment: reviewID=%d", reviewID) + + // Fallback: If reviewID is 0, try to fetch the latest review from the API + if reviewID == 0 { + log.Printf("[INFO] Gitea enrichment: reviewID missing, fetching latest review for PR #%d", prNumber) + latestReview, err := p.fetchLatestReview(baseURL, token.PatToken, owner, repo, prNumber) + if err != nil { + log.Printf("[WARN] Failed to fetch latest review for enrichment: %v", err) + } else if latestReview != nil { + reviewID = latestReview.ID + log.Printf("[INFO] Gitea enrichment: found latest review ID %d", reviewID) + } + } + + if reviewID > 0 { + log.Printf("[DEBUG] Gitea enrichment: fetching comments for review %d", reviewID) + comments, err := p.fetchReviewComments(baseURL, token.PatToken, owner, repo, prNumber, reviewID) + if err != nil { + log.Printf("[WARN] Failed to fetch review comments for enrichment: %v", err) + } else { + // Get bot info to know what mention to look for + botInfo, _ := p.GetBotUserInfo(event.Repository) + mentionName := "livereview" + if botInfo != nil && botInfo.Username != "" { + mentionName = botInfo.Username + } + + mentionPattern := "@" + strings.ToLower(mentionName) + log.Printf("[DEBUG] Gitea enrichment: scanning %d comments for mention '%s'", len(comments), mentionPattern) + + for _, c := range comments { + if strings.Contains(strings.ToLower(c.Body), mentionPattern) { + log.Printf("[INFO] Gitea enrichment: found mention in inline comment %d, promoting it", c.ID) + // Update the event's comment to be this inline comment + event.Comment.ID = strconv.FormatInt(c.ID, 10) + event.Comment.Body = c.Body + event.Comment.CreatedAt = c.CreatedAt + event.Comment.UpdatedAt = c.UpdatedAt + + // Save the review ID so the output client knows how to route this + if event.Comment.Metadata == nil { + event.Comment.Metadata = map[string]interface{}{} + } + event.Comment.Metadata["review_id"] = reviewID + + // Add position info so LR knows which file/line we are talking about + if c.Path != "" { + // 1. Try the direct line field first; start with the side Gitea provides + lineNumber := c.Line + lineType := convertGiteaSideToLineType(c.Side) + + // 2. If Gitea gave us 0, calculate both line number AND side from the diff_hunk. + // Gitea omits `side` for many comment types (deleted lines in particular), + // so we infer it from whether the last real line in the hunk is a deletion. + if lineNumber == 0 && c.DiffHunk != "" { + lineNumber, lineType = getExactLineFromHunk(c.DiffHunk, c.Side) + log.Printf("[DEBUG] Extracted exact line %d (lineType=%s) from diff_hunk for comment %d", lineNumber, lineType, c.ID) + } + + event.Comment.Position = &coreprocessor.UnifiedPositionV2{ + FilePath: c.Path, + LineNumber: lineNumber, + LineType: lineType, + } + } + break // Take the first mention we find + } + } + } + } + } + } + return nil } @@ -574,6 +661,41 @@ func (p *GiteaV2Provider) fetchReviewComments(baseURL, token, owner, repo string return comments, nil } +func (p *GiteaV2Provider) fetchLatestReview(baseURL, token, owner, repo string, prNumber int) (*GiteaReview, error) { + apiURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", baseURL, owner, repo, prNumber) + + req, err := networkgitea.NewRequestWithContext(context.Background(), "GET", apiURL, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("Authorization", "token "+token) + req.Header.Set("Accept", "application/json") + + resp, err := networkgitea.Do(p.botUserHTTPClient, req) + if err != nil { + return nil, fmt.Errorf("failed to fetch reviews: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("gitea API returned status %d", resp.StatusCode) + } + + var reviews []GiteaReview + if err := json.NewDecoder(resp.Body).Decode(&reviews); err != nil { + return nil, fmt.Errorf("failed to decode reviews: %w", err) + } + + if len(reviews) == 0 { + return nil, nil + } + + // Reviews are usually returned in chronological order or ID order. + // We want the most recent one. + return &reviews[len(reviews)-1], nil +} + func extractRepoFullNameFromMetadata(metadata map[string]interface{}) (string, error) { if metadata == nil { return "", fmt.Errorf("metadata is nil") @@ -639,6 +761,94 @@ func (p *GiteaV2Provider) ValidateWebhookSignature(connectorID int64, headers ma return true } +// getExactLineFromHunk calculates the exact line number AND line type from Gitea's diff_hunk. +// The hunk header format is: @@ -oldStart[,oldCount] +newStart[,newCount] @@[optional context] +// Returns (lineNumber, lineType) where lineType is "old" or "new". +// When side is empty (Gitea omits it for many comment types), lineType is inferred from +// whether the last real code line in the hunk is a deletion (-) or not. +func getExactLineFromHunk(hunk, side string) (int, string) { + if hunk == "" { + return 0, "new" + } + + log.Printf("[DEBUG] getExactLineFromHunk: input side=%q, hunk=%q", side, hunk) + + // Flexible regex: allows any whitespace between tokens and ignores optional trailing context + // after the closing @@. Handles both "@@ -6,2 +6,4 @@" and "@@ -6 +6 @@ funcName" etc. + re := regexp.MustCompile(`@@\s*-(\d+)(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s*@@`) + matches := re.FindStringSubmatch(hunk) + if len(matches) < 3 { + log.Printf("[DEBUG] getExactLineFromHunk: regex did not match hunk header") + return 0, "new" + } + + var oldStart, newStart int + fmt.Sscanf(matches[1], "%d", &oldStart) + fmt.Sscanf(matches[2], "%d", &newStart) + log.Printf("[DEBUG] getExactLineFromHunk: oldStart=%d newStart=%d", oldStart, newStart) + + newLine := newStart - 1 + oldLine := oldStart - 1 + + // Normalize \r\n to \n before splitting to handle Windows-style line endings + normalized := strings.ReplaceAll(hunk, "\r\n", "\n") + lines := strings.Split(normalized, "\n") + + // Skip the first line (the @@ header) and count the actual code lines. + // NOTE: Gitea's diff_hunk always ends with a trailing \n, so Split produces a + // trailing "" entry. We must count it (context +1) as it provides the offset to + // land on the actual commented line. BUT we must NOT let it overwrite + // lastRealLineType — the trailing empty is not a real code line, and overwriting + // would hide that the actual last line was a deletion, causing wrong side inference. + lastRealLineType := "context" + for i := 1; i < len(lines); i++ { + line := strings.TrimRight(lines[i], "\r") // strip any stray \r + var prefix string + if strings.HasPrefix(line, "+") { + newLine++ + lastRealLineType = "added" + prefix = "+" + } else if strings.HasPrefix(line, "-") { + oldLine++ + lastRealLineType = "deleted" + prefix = "-" + } else { // context lines AND trailing empty: both bump both counters + newLine++ + oldLine++ + if line != "" { // only real context lines update the type + lastRealLineType = "context" + } + prefix = " " + } + log.Printf("[DEBUG] getExactLineFromHunk: line[%d] prefix=%q content=%q → newLine=%d oldLine=%d lastRealLineType=%s", + i, prefix, line, newLine, oldLine, lastRealLineType) + } + + log.Printf("[DEBUG] getExactLineFromHunk: final newLine=%d oldLine=%d lastRealLineType=%s side=%q", + newLine, oldLine, lastRealLineType, side) + + // Explicit side wins. + if strings.ToUpper(side) == "LEFT" || strings.ToLower(side) == "previous" { + log.Printf("[DEBUG] getExactLineFromHunk: explicit LEFT/previous → oldLine=%d", oldLine) + return oldLine, "old" + } + if strings.ToUpper(side) == "RIGHT" || strings.ToLower(side) == "proposed" { + log.Printf("[DEBUG] getExactLineFromHunk: explicit RIGHT/proposed → newLine=%d", newLine) + return newLine, "new" + } + + // side is empty/unknown (Gitea omits it for many comment types). + // Infer from the last real code line in the hunk: + // - ends on a deleted line → comment is on old side → return oldLine, "old" + // - ends on added/context → comment is on new side → return newLine, "new" + if lastRealLineType == "deleted" { + log.Printf("[DEBUG] getExactLineFromHunk: inferred old side (lastRealLineType=deleted) → oldLine=%d", oldLine) + return oldLine, "old" + } + log.Printf("[DEBUG] getExactLineFromHunk: inferred new side (lastRealLineType=%s) → newLine=%d", lastRealLineType, newLine) + return newLine, "new" +} + // IntegrationToken represents a token from the database type IntegrationToken struct { ID int64 diff --git a/internal/provider_input/gitea/gitea_types.go b/internal/provider_input/gitea/gitea_types.go index 79f9c60d..76ced88c 100644 --- a/internal/provider_input/gitea/gitea_types.go +++ b/internal/provider_input/gitea/gitea_types.go @@ -201,6 +201,7 @@ type GiteaReviewComment struct { Line int `json:"line"` Side string `json:"side"` // LEFT or RIGHT CommitID string `json:"commit_id"` + DiffHunk string `json:"diff_hunk"` CreatedAt string `json:"created_at"` UpdatedAt string `json:"updated_at"` } diff --git a/internal/provider_output/gitea/api_client.go b/internal/provider_output/gitea/api_client.go index 0cf63791..60f3ecf0 100644 --- a/internal/provider_output/gitea/api_client.go +++ b/internal/provider_output/gitea/api_client.go @@ -97,10 +97,18 @@ func (c *APIClient) PostCommentReply(event *UnifiedWebhookEventV2, token, replyT } } - // If webhook lacks review context but we have comment_id, fetch from API - // Gitea webhooks for replies to inline comments don't include position/review_id + // Always enrich when reviewID==0: Gitea sends identical issue_comment webhooks for + // true general PR comments AND inline thread replies. The only way to distinguish + // them is via the API. enrichCommentMetadata is a no-op (leaves Position nil) when + // the comment is not found in any review → falls through to general comment path. + // replyCommentID identifies the specific comment being replied to within a review thread. + // It is collected here for completeness and potential future use (e.g. an in_reply_to field + // if Gitea's API exposes one), but is not currently written into the multipart form because + // Gitea's web UI uses the review-level ID (reviewID) for thread attachment, not the comment ID. + var replyCommentID int64 if reviewID == 0 && event.Comment.ID != "" { - log.Printf("[DEBUG] Webhook lacks review context (review_id=0), attempting metadata enrichment for comment_id=%s", event.Comment.ID) + log.Printf("[DEBUG] reviewID=0, attempting enrichment for comment_id=%s", event.Comment.ID) + // Use username/password to create temporary PAT for API call if PAT is invalid enrichToken := token if creds.Username != "" && creds.Password != "" { @@ -117,15 +125,26 @@ func (c *APIClient) PostCommentReply(event *UnifiedWebhookEventV2, token, replyT } else { log.Printf("[DEBUG] Metadata enrichment completed") } - // Re-extract review_id after enrichment + // Re-extract review_id and reply_comment_id after enrichment if event.Comment.Metadata != nil { if rid, ok := event.Comment.Metadata["review_id"].(int64); ok && rid > 0 { reviewID = rid - log.Printf("[DEBUG] Extracted review_id after enrichment: %d", reviewID) } else if ridFloat, ok := event.Comment.Metadata["review_id"].(float64); ok && ridFloat > 0 { reviewID = int64(ridFloat) - log.Printf("[DEBUG] Extracted review_id after enrichment (float): %d", reviewID) } + if rcid, ok := event.Comment.Metadata["reply_comment_id"].(int64); ok && rcid > 0 { + replyCommentID = rcid + } else if rcidFloat, ok := event.Comment.Metadata["reply_comment_id"].(float64); ok && rcidFloat > 0 { + replyCommentID = int64(rcidFloat) + } + log.Printf("[DEBUG] Extracted after enrichment: review_id=%d, reply_comment_id=%d", reviewID, replyCommentID) + } + } else if event.Comment.Metadata != nil { + // Even if reviewID was present, try to get replyCommentID from metadata + if rcid, ok := event.Comment.Metadata["reply_comment_id"].(int64); ok && rcid > 0 { + replyCommentID = rcid + } else if rcidFloat, ok := event.Comment.Metadata["reply_comment_id"].(float64); ok && rcidFloat > 0 { + replyCommentID = int64(rcidFloat) } } @@ -141,11 +160,26 @@ func (c *APIClient) PostCommentReply(event *UnifiedWebhookEventV2, token, replyT // Route based on whether this is a review comment or general comment if reviewID > 0 && event.Comment.Position != nil { // Inline review comment - use multipart form - return c.postInlineCommentReply(baseURL, owner, repo, event, replyText, reviewID, creds.Username, creds.Password) + return c.postInlineCommentReply(baseURL, owner, repo, event, replyText, reviewID, replyCommentID, creds.Username, creds.Password) + } + + // General comment on issue/PR: quote the original comment and tag the author + var quotedBody strings.Builder + if event.Comment.Author.Username != "" { + quotedBody.WriteString(fmt.Sprintf("@%s\n", event.Comment.Author.Username)) } + if event.Comment.Body != "" { + for _, line := range strings.Split(strings.TrimSpace(event.Comment.Body), "\n") { + quotedBody.WriteString("> " + line + "\n") + } + } + if quotedBody.Len() > 0 { + quotedBody.WriteString("\n") + } + + formattedReply := quotedBody.String() + strings.TrimSpace(replyText) - // General comment on issue/PR - return c.postGeneralComment(baseURL, owner, repo, event.MergeRequest.Number, token, replyText) + return c.postGeneralComment(baseURL, owner, repo, event.MergeRequest.Number, token, formattedReply) } // postGeneralComment posts a general comment to an issue or PR @@ -164,7 +198,7 @@ func (c *APIClient) postGeneralComment(baseURL, owner, repo string, prNumber int } // postInlineCommentReply posts a reply to an inline code review comment using multipart form. -func (c *APIClient) postInlineCommentReply(baseURL, owner, repo string, event *UnifiedWebhookEventV2, replyText string, reviewID int64, username, password string) error { +func (c *APIClient) postInlineCommentReply(baseURL, owner, repo string, event *UnifiedWebhookEventV2, replyText string, reviewID, replyCommentID int64, username, password string) error { if username == "" || password == "" { return fmt.Errorf("inline comment reply requires username/password credentials") } @@ -181,7 +215,7 @@ func (c *APIClient) postInlineCommentReply(baseURL, owner, repo string, event *U return fmt.Errorf("inline comment reply requires valid line number (got: %d)", event.Comment.Position.LineNumber) } - return c.postInlineCommentReplyMultipart(baseURL, owner, repo, event.MergeRequest.Number, event, replyText, reviewID, username, password) + return c.postInlineCommentReplyMultipart(baseURL, owner, repo, event.MergeRequest.Number, event, replyText, reviewID, replyCommentID, username, password) } // enrichCommentMetadata fetches review context by scanning all reviews in the PR. @@ -189,7 +223,11 @@ func (c *APIClient) postInlineCommentReply(baseURL, owner, repo string, event *U // We scan all reviews to find the inline comment being replied to and extract its context. func (c *APIClient) enrichCommentMetadata(baseURL, owner, repo string, event *UnifiedWebhookEventV2, token string) error { prNumber := event.MergeRequest.Number - log.Printf("[DIAG] enrichCommentMetadata ENTRY: pr=%d, comment_id=%s", prNumber, event.Comment.ID) + targetID, parseErr := strconv.ParseInt(event.Comment.ID, 10, 64) + if parseErr != nil { + return fmt.Errorf("invalid comment ID %q: %w", event.Comment.ID, parseErr) + } + log.Printf("[DIAG] enrichCommentMetadata ENTRY: pr=%d, target_comment_id=%d", prNumber, targetID) // Fetch all reviews for this PR reviewsURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", baseURL, owner, repo, prNumber) @@ -227,20 +265,26 @@ func (c *APIClient) enrichCommentMetadata(baseURL, owner, repo string, event *Un return fmt.Errorf("failed to decode reviews: %w", err) } - log.Printf("[DIAG] Found %d reviews, scanning for inline comments", len(reviews)) + log.Printf("[DIAG] Found %d reviews, scanning for target comment %d", len(reviews), targetID) - // Scan each review's comments to find the most recent inline comment - var latestComment map[string]interface{} - latestTime := "" - totalCommentsScanned := 0 - inlineCommentsFound := 0 + // Build full index: commentID → {reviewID, path, position, originalPosition, line} + // Gitea sets position=0 when a comment becomes "outdated" (new commits pushed), + // but original_position retains the original diff position and is always non-zero. + type entry struct { + ReviewID int64 + Path string + Position float64 // current diff position (0 when outdated) + OriginalPosition float64 // original diff position (always set for inline comments) + Line float64 // actual file line (null in this Gitea version) + InReplyTo int64 // ID of the comment being replied to (null in this Gitea version) + } + index := make(map[int64]entry) + // reviewRootComment tracks the LOWEST comment ID per review. + // Gitea's multipart form `reply` field expects this root comment ID, not the review ID. + // Using the review ID creates a new thread; using the root comment ID appends to the thread. + reviewRootComment := make(map[int64]int64) // reviewID → root comment ID for _, review := range reviews { - if review.CommentsCount == 0 { - log.Printf("[DIAG] Review %d has no comments, skipping", review.ID) - continue - } - commentsURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments", baseURL, owner, repo, prNumber, review.ID) log.Printf("[DIAG] Fetching comments from review %d: %s", review.ID, commentsURL) @@ -273,68 +317,207 @@ func (c *APIClient) enrichCommentMetadata(baseURL, owner, repo string, event *Un cresp.Body.Close() log.Printf("[DIAG] Review %d has %d comments", review.ID, len(comments)) - totalCommentsScanned += len(comments) - - // Find latest inline comment (has path and position) - matching Python script logic for _, cmt := range comments { + id, _ := cmt["id"].(float64) path, _ := cmt["path"].(string) position, _ := cmt["position"].(float64) - created, _ := cmt["created_at"].(string) - cmtID, _ := cmt["id"].(float64) - - log.Printf("[DIAG] Comment %.0f: path=%s, position=%.0f, created=%s", cmtID, path, position, created) - - // Python script: filter by path only, position is used for form submission - if path != "" && position > 0 { - inlineCommentsFound++ - if created > latestTime { - log.Printf("[DIAG] New latest inline comment: %.0f (prev_time=%s, new_time=%s)", cmtID, latestTime, created) - latestComment = cmt - latestTime = created - } - } else { - log.Printf("[DIAG] Comment %.0f is not inline (no path or position=0)", cmtID) + originalPosition, _ := cmt["original_position"].(float64) + line, _ := cmt["line"].(float64) + originalLine, _ := cmt["original_line"].(float64) + if line == 0 { + line = originalLine } + inReplyTo, _ := cmt["in_reply_to"].(float64) + prReviewID, _ := cmt["pull_request_review_id"].(float64) + rid := int64(prReviewID) + if rid == 0 { + rid = review.ID + } + + // Extract side information for LineType determination + side, _ := cmt["side"].(string) + if side == "" { + side = "RIGHT" // default to new side + } + + index[int64(id)] = entry{ + ReviewID: rid, + Path: path, + Position: position, + OriginalPosition: originalPosition, + Line: line, + InReplyTo: int64(inReplyTo), + } + + // Store side information for LineType determination + if event.Comment.ID == fmt.Sprintf("%.0f", id) { + log.Printf("[DEBUG] Found target comment in API response: side=%s, path=%s, position=%.0f", side, path, position) + event.Comment.Metadata["original_side"] = side + } + // Track the lowest comment ID per review (= thread root for the reply field) + cmtID := int64(id) + if existing, ok := reviewRootComment[rid]; !ok || cmtID < existing { + reviewRootComment[rid] = cmtID + } + log.Printf("[DIAG] Indexed comment %.0f: review_id=%d, path=%s, pos=%.0f, orig_pos=%.0f, line=%.0f, in_reply_to=%.0f", + id, rid, path, position, originalPosition, line, inReplyTo) } } - log.Printf("[DIAG] Scan complete: total_comments=%d, inline_comments=%d, latest_found=%v", - totalCommentsScanned, inlineCommentsFound, latestComment != nil) + // NOTE: The flat /pulls/{index}/comments endpoint returns 404 on this Gitea instance + // (confirmed — not a PAT scope issue, the endpoint is not accessible). + // We rely entirely on per-review /reviews/{id}/comments which gives us original_position. - if latestComment == nil { - log.Printf("[DEBUG] No inline review comments found in PR %d", prNumber) + // Step 1: Is our target comment in any review at all? + target, found := index[targetID] + if !found { + log.Printf("[DIAG] Comment %d not found in any review → true general PR comment, no enrichment", targetID) return nil } + log.Printf("[DIAG] Target comment %d found in reviews: review_id=%d, path=%s, position=%.0f, line=%.0f", + targetID, target.ReviewID, target.Path, target.Position, target.Line) - // Populate metadata from the latest inline comment (use position field like Python script) - reviewID, _ := latestComment["pull_request_review_id"].(float64) - path, _ := latestComment["path"].(string) - position, _ := latestComment["position"].(float64) - cmtID, _ := latestComment["id"].(float64) + // effectiveLine returns the best position to use for the multipart reply form. + // Priority: position (current diff pos) → original_position (pre-outdated pos) → line (file line). + // Gitea sets position=0 when commits are pushed after the comment, but original_position + // retains the value needed to correctly anchor the multipart reply to the right thread. + effectiveLine := func(e entry) float64 { + if e.Position > 0 { + return e.Position + } + if e.OriginalPosition > 0 { + return e.OriginalPosition + } + return e.Line + } - log.Printf("[DIAG] Using latest inline comment: id=%.0f, review_id=%.0f, path=%s, position=%.0f", - cmtID, reviewID, path, position) + // Step 2: If target itself has an inline anchor, use it directly. + if ef := effectiveLine(target); ef > 0 { + event.Comment.Metadata["review_id"] = target.ReviewID + if rootCmtID, ok := reviewRootComment[target.ReviewID]; ok { + event.Comment.Metadata["reply_comment_id"] = rootCmtID + } + // Determine LineType: Use position=0 to identify deleted lines (more reliable than Gitea's side field) + lineType := "new" // default for new lines + if target.Position == 0 && target.OriginalPosition > 0 { + // position=0 means the line was deleted/modified after the comment was made + // This indicates a deleted line + lineType = "old" + log.Printf("[DEBUG] Detected deleted line: position=0, original_position=%.0f -> LineType=old", target.OriginalPosition) + } else { + log.Printf("[DEBUG] Detected new/modified line: position=%.0f -> LineType=new", target.Position) + } - // Populate review_id into event metadata for routing decision - if reviewID > 0 { - if event.Comment.Metadata == nil { - event.Comment.Metadata = make(map[string]interface{}) + event.Comment.Position = &UnifiedPositionV2{ + FilePath: target.Path, + LineNumber: int(ef), + LineType: lineType, } - event.Comment.Metadata["review_id"] = int64(reviewID) - log.Printf("[DIAG] Populated review_id in metadata: %.0f", reviewID) + + log.Printf("[DEBUG] Set comment position: path=%s, line=%d, lineType=%s", target.Path, int(ef), lineType) + log.Printf("[DIAG] EXIT: target has anchor, review_id=%d, reply_comment_id=%v, path=%s, line=%d", + target.ReviewID, event.Comment.Metadata["reply_comment_id"], target.Path, int(ef)) + return nil } - if path != "" && position > 0 { - event.Comment.Position = &UnifiedPositionV2{ - FilePath: path, - LineNumber: int(position), - LineType: "new", + // Step 3: Target has no anchor (position=0, line=0). Trace the InReplyTo chain. + log.Printf("[DIAG] Step 3: Tracing InReplyTo chain for comment %d", targetID) + currID := targetID + visited := make(map[int64]bool) + effectiveReviewID := target.ReviewID + + for currID != 0 && !visited[currID] { + visited[currID] = true + curr, exists := index[currID] + if !exists { + log.Printf("[DIAG] Trace broke at comment %d (not in index)", currID) + break + } + + // Update effectiveReviewID as we climb the chain (prefer non-zero) + if curr.ReviewID > 0 { + effectiveReviewID = curr.ReviewID + } + + if ef := effectiveLine(curr); ef > 0 { + event.Comment.Metadata["review_id"] = curr.ReviewID + if rootCmtID, ok := reviewRootComment[curr.ReviewID]; ok { + event.Comment.Metadata["reply_comment_id"] = rootCmtID + } + event.Comment.Position = &UnifiedPositionV2{ + FilePath: curr.Path, + LineNumber: int(ef), + LineType: "new", + } + log.Printf("[DIAG] EXIT: traced to anchor at comment %d, review_id=%d, reply_comment_id=%v, path=%s, line=%d", + currID, curr.ReviewID, event.Comment.Metadata["reply_comment_id"], curr.Path, int(ef)) + return nil + } + currID = curr.InReplyTo + } + + // Step 4: Fallback - if no chain found, use the closest review heuristic. + // We use effectiveReviewID to constrain the search to the correct discussion thread + // when multiple discussions exist on the same file path. + log.Printf("[DIAG] Step 4: Fallback to closest-review heuristic for comment %d (effectiveReviewID=%d)", targetID, effectiveReviewID) + if target.Path != "" { + type candidate struct { + cmtID int64 + reviewID int64 + line float64 + } + var candidates []candidate + for cmtID, e := range index { + ef := effectiveLine(e) + // Constraint: same path AND (we don't know the review OR it matches exactly) + if e.Path == target.Path && ef > 0 { + if effectiveReviewID == 0 || e.ReviewID == effectiveReviewID { + candidates = append(candidates, candidate{cmtID, e.ReviewID, ef}) + } + } + } + log.Printf("[DIAG] Candidates with anchor on path=%s and reviewID=%d: %d", target.Path, effectiveReviewID, len(candidates)) + + if len(candidates) > 0 { + // Pick closest review_id ≤ effectiveReviewID (or closest overall if all are later). + var best *candidate + for i := range candidates { + c := &candidates[i] + if effectiveReviewID == 0 || c.reviewID <= effectiveReviewID { + if best == nil || c.reviewID > best.reviewID { + best = c + } + } + } + if best == nil { + // Fallback: pick the one with lowest review_id overall + for i := range candidates { + c := &candidates[i] + if best == nil || c.reviewID < best.reviewID { + best = c + } + } + } + if best != nil { + e := index[best.cmtID] + ef := effectiveLine(e) + event.Comment.Metadata["review_id"] = e.ReviewID + if rootCmtID, ok := reviewRootComment[e.ReviewID]; ok { + event.Comment.Metadata["reply_comment_id"] = rootCmtID + } + event.Comment.Position = &UnifiedPositionV2{ + FilePath: e.Path, + LineNumber: int(ef), + LineType: "new", + } + log.Printf("[DIAG] EXIT: fallback to candidate %d, review_id=%d, reply_comment_id=%v, path=%s, line=%d", + best.cmtID, e.ReviewID, event.Comment.Metadata["reply_comment_id"], e.Path, int(ef)) + return nil + } } - log.Printf("[DIAG] Populated position: path=%s, position=%.0f", path, position) } - log.Printf("[DIAG] enrichCommentMetadata EXIT: SUCCESS (review_id=%.0f, has_position=%v)", - reviewID, event.Comment.Position != nil) + log.Printf("[DIAG] Comment %d in review but no inline position found → routing as general comment", targetID) return nil } @@ -620,9 +803,13 @@ func (c *APIClient) postToGiteaAPI(apiURL, token string, requestBody interface{} // postInlineCommentReplyMultipart emulates the browser form submission used by Gitea when replying inline. // Requires username/password (from packed connector) to obtain session + CSRF. -func (c *APIClient) postInlineCommentReplyMultipart(baseURL, owner, repo string, prNumber int, event *UnifiedWebhookEventV2, replyText string, reviewID int64, username, password string) error { - log.Printf("[DIAG] postInlineCommentReplyMultipart ENTRY: baseURL=%s, owner=%s, repo=%s, prNumber=%d, reviewID=%d, replyTextLen=%d", - baseURL, owner, repo, prNumber, reviewID, len(replyText)) +// +// replyCommentID is accepted for diagnostic logging and future use (e.g. an in_reply_to field if +// Gitea's API exposes one). It is intentionally NOT written to the multipart form: Gitea's web UI +// uses the review-level ID (reviewID) in the "reply" field for thread attachment, not the comment ID. +func (c *APIClient) postInlineCommentReplyMultipart(baseURL, owner, repo string, prNumber int, event *UnifiedWebhookEventV2, replyText string, reviewID, replyCommentID int64, username, password string) error { + log.Printf("[DIAG] postInlineCommentReplyMultipart ENTRY: baseURL=%s, owner=%s, repo=%s, prNumber=%d, reviewID=%d, replyCommentID=%d, replyTextLen=%d", + baseURL, owner, repo, prNumber, reviewID, replyCommentID, len(replyText)) log.Printf("[DIAG] Event comment ID=%s, author=%s", event.Comment.ID, event.Comment.Author.Username) if username == "" || password == "" { return fmt.Errorf("multipart fallback requires username/password in connector token") @@ -684,28 +871,49 @@ func (c *APIClient) postInlineCommentReplyMultipart(baseURL, owner, repo string, // Position validated in postInlineCommentReply line := strconv.Itoa(event.Comment.Position.LineNumber) path := event.Comment.Position.FilePath - commit := "" - if sha, ok := event.MergeRequest.Metadata["head_sha"].(string); ok { - commit = sha + + // Determine side based on line type: 'previous' for deleted lines, 'proposed' for new lines + side := "proposed" // default for new lines + + // Debug logging to understand the data structure + log.Printf("[DEBUG] Comment position data: LineType=%s, LineNumber=%d, FilePath=%s", + event.Comment.Position.LineType, event.Comment.Position.LineNumber, event.Comment.Position.FilePath) + + if event.Comment.Position.Metadata != nil { + if originalSide, exists := event.Comment.Position.Metadata["original_side"]; exists { + log.Printf("[DEBUG] Original Gitea side from metadata: %v", originalSide) + } } - log.Printf("[DEBUG] Posting inline reply: line=%s, path=%s, reviewID=%d", line, path, reviewID) + if event.Comment.Position.LineType == "old" { + side = "previous" // for deleted lines + log.Printf("[DEBUG] Detected deleted line, setting side=previous") + } else { + log.Printf("[DEBUG] Using default side=proposed for lineType=%s", event.Comment.Position.LineType) + } - // Build multipart form mirroring the working curl + log.Printf("[DEBUG] Final inline reply: line=%s, path=%s, reviewID=%d, replyCommentID=%d, side=%s, lineType=%s", + line, path, reviewID, replyCommentID, side, event.Comment.Position.LineType) + + // Build multipart form mirroring the working Python implementation exactly var buf bytes.Buffer mw := multipart.NewWriter(&buf) + fields := map[string]string{ "_csrf": csrf, "origin": "timeline", - "latest_commit_id": commit, - "side": "proposed", + "latest_commit_id": "", // Empty string matches working Python implementation + "side": side, "line": line, "path": path, "diff_start_cid": "", "diff_end_cid": "", "diff_base_cid": "", "content": replyText, - "reply": strconv.FormatInt(reviewID, 10), + // "reply" is Gitea's internal web-form field for attaching a comment to a review thread. + // It expects the review-level ID (reviewID), NOT the individual comment ID (replyCommentID). + // Using replyCommentID here would attach the reply to the wrong thread or cause a 404. + "reply": strconv.FormatInt(reviewID, 10), "single_review": "true", } for k, v := range fields { diff --git a/internal/providers/gitea/gitea_provider.go b/internal/providers/gitea/gitea_provider.go index 0ac41f03..5b8bb469 100644 --- a/internal/providers/gitea/gitea_provider.go +++ b/internal/providers/gitea/gitea_provider.go @@ -11,9 +11,11 @@ import ( "regexp" "strconv" "strings" + "time" "github.com/livereview/internal/aisanitize" "github.com/livereview/internal/providers" + networkgitea "github.com/livereview/network/providers/gitea" "github.com/livereview/pkg/models" ) @@ -50,7 +52,7 @@ func NewProvider(cfg Config) (*Provider, error) { token: tok, username: user, password: pass, - httpClient: &http.Client{}, + httpClient: networkgitea.NewHTTPClient(30 * time.Second), }, nil } @@ -104,7 +106,7 @@ func (p *Provider) Configure(config map[string]interface{}) error { p.baseURL = NormalizeGiteaBaseURL(base) p.token = token if p.httpClient == nil { - p.httpClient = &http.Client{} + p.httpClient = networkgitea.NewHTTPClient(30 * time.Second) } p.session = nil // reset session on reconfigure return nil @@ -118,13 +120,13 @@ func (p *Provider) GetMergeRequestDetails(ctx context.Context, mrURL string) (*p } apiURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", apiBase, owner, repo, index) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + req, err := networkgitea.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) if err != nil { return nil, fmt.Errorf("failed to build request: %w", err) } p.applyAuthHeaders(req) - resp, err := p.httpClient.Do(req) + resp, err := networkgitea.Do(p.httpClient, req) if err != nil { return nil, fmt.Errorf("failed to fetch pull request: %w", err) } @@ -199,13 +201,13 @@ func (p *Provider) GetMergeRequestChanges(ctx context.Context, prID string) ([]* // Fallback to files endpoint if unified diff is unavailable apiURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%s/files", apiBase, owner, repo, number) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + req, err := networkgitea.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) if err != nil { return nil, fmt.Errorf("failed to build request: %w", err) } p.applyAuthHeaders(req) - resp, err := p.httpClient.Do(req) + resp, err := networkgitea.Do(p.httpClient, req) if err != nil { return nil, fmt.Errorf("failed to fetch pull request files: %w", err) } @@ -244,13 +246,13 @@ func (p *Provider) GetMergeRequestChanges(ctx context.Context, prID string) ([]* func (p *Provider) fetchDiffAsUnified(ctx context.Context, apiBase, owner, repo, number string) ([]*models.CodeDiff, error) { // Gitea supports /repos/{owner}/{repo}/pulls/{index}.diff or .patch apiURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%s.diff", apiBase, owner, repo, number) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + req, err := networkgitea.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) if err != nil { return nil, fmt.Errorf("failed to build diff request: %w", err) } p.applyAuthHeaders(req) - resp, err := p.httpClient.Do(req) + resp, err := networkgitea.Do(p.httpClient, req) if err != nil { return nil, fmt.Errorf("failed to fetch diff: %w", err) } @@ -269,15 +271,41 @@ func (p *Provider) fetchDiffAsUnified(ctx context.Context, apiBase, owner, repo, return parseUnifiedDiff(string(body)), nil } +// formatGiteaComment creates a consistently formatted comment for Gitea +// with severity information and suggestions properly formatted +func formatGiteaComment(ctx context.Context, comment *models.ReviewComment) string { + safeContent, _ := aisanitize.SanitizationPostflight(ctx, comment.Content) + + safeSuggestions := make([]string, 0, len(comment.Suggestions)) + for _, suggestion := range comment.Suggestions { + safeSuggestion, _ := aisanitize.SanitizationPostflight(ctx, suggestion) + safeSuggestions = append(safeSuggestions, safeSuggestion) + } + + formattedComment := safeContent + if comment.Severity != "" { + formattedComment = fmt.Sprintf("**Severity: %s**\n\n%s", comment.Severity, formattedComment) + } + + if len(safeSuggestions) > 0 { + formattedComment += "\n\n**Suggestions:**\n" + for i, suggestion := range safeSuggestions { + formattedComment += fmt.Sprintf("%d. %s\n", i+1, suggestion) + } + } + + return formattedComment +} + // PostComment posts a comment on a PR. Supports inline (file/line) comments and general comments. func (p *Provider) PostComment(ctx context.Context, prID string, comment *models.ReviewComment) error { if comment == nil { return fmt.Errorf("comment is required") } - safeContent, _ := aisanitize.SanitizationPostflight(ctx, comment.Content) + formattedContent := formatGiteaComment(ctx, comment) safeComment := *comment - safeComment.Content = safeContent + safeComment.Content = formattedContent parts := strings.Split(prID, "/") if len(parts) != 3 { @@ -312,14 +340,14 @@ func (p *Provider) PostComment(ctx context.Context, prID string, comment *models payload := map[string]string{"body": safeComment.Content} body, _ := json.Marshal(payload) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, apiURL, strings.NewReader(string(body))) + req, err := networkgitea.NewRequestWithContext(ctx, http.MethodPost, apiURL, strings.NewReader(string(body))) if err != nil { return fmt.Errorf("failed to build request: %w", err) } p.applyAuthHeaders(req) req.Header.Set("Content-Type", "application/json") - resp, err := p.httpClient.Do(req) + resp, err := networkgitea.Do(p.httpClient, req) if err != nil { return fmt.Errorf("failed to post comment: %w", err) } @@ -374,14 +402,14 @@ func (p *Provider) postInlineViaSession(ctx context.Context, apiBase, owner, rep form.Set("content", comment.Content) form.Set("single_review", "true") - postReq, err := http.NewRequestWithContext(ctx, http.MethodPost, commentURL, strings.NewReader(form.Encode())) + postReq, err := networkgitea.NewRequestWithContext(ctx, http.MethodPost, commentURL, strings.NewReader(form.Encode())) if err != nil { return fmt.Errorf("failed to build session inline comment request: %w", err) } postReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") postReq.Header.Set("X-CSRF-Token", p.session.csrf) - resp, err := p.session.client.Do(postReq) + resp, err := networkgitea.Do(p.session.client, postReq) if err != nil { return fmt.Errorf("failed to post inline via session: %w", err) } @@ -391,7 +419,7 @@ func (p *Provider) postInlineViaSession(ctx context.Context, apiBase, owner, rep if err := p.relogin(ctx); err != nil { return fmt.Errorf("session relogin failed: %w", err) } - resp, err = p.session.client.Do(postReq) + resp, err = networkgitea.Do(p.session.client, postReq) if err != nil { return fmt.Errorf("failed to post inline after relogin: %w", err) } @@ -423,14 +451,14 @@ func (p *Provider) ensureSession(ctx context.Context) error { return fmt.Errorf("session credentials not provided for Gitea inline fallback") } jar, _ := cookiejar.New(nil) - cli := &http.Client{Jar: jar} + cli := networkgitea.NewHTTPClientWithJar(30*time.Second, jar) loginURL := fmt.Sprintf("%s/user/login", p.baseURL) - getReq, err := http.NewRequestWithContext(ctx, http.MethodGet, loginURL, nil) + getReq, err := networkgitea.NewRequestWithContext(ctx, http.MethodGet, loginURL, nil) if err != nil { return fmt.Errorf("failed to build login GET: %w", err) } - resp, err := cli.Do(getReq) + resp, err := networkgitea.Do(cli, getReq) if err != nil { return fmt.Errorf("failed to fetch login page: %w", err) } @@ -450,13 +478,13 @@ func (p *Provider) ensureSession(ctx context.Context) error { form.Set("password", p.password) form.Set("remember", "on") - postReq, err := http.NewRequestWithContext(ctx, http.MethodPost, loginURL, strings.NewReader(form.Encode())) + postReq, err := networkgitea.NewRequestWithContext(ctx, http.MethodPost, loginURL, strings.NewReader(form.Encode())) if err != nil { return fmt.Errorf("failed to build login POST: %w", err) } postReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err = cli.Do(postReq) + resp, err = networkgitea.Do(cli, postReq) if err != nil { return fmt.Errorf("failed to execute login POST: %w", err) } @@ -527,13 +555,13 @@ func cookieValue(jar http.CookieJar, rawURL, name string) string { func (p *Provider) fetchPullRequest(ctx context.Context, owner, repo, number string) (*pullRequest, error) { apiURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%s", p.baseURL, owner, repo, number) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + req, err := networkgitea.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) if err != nil { return nil, fmt.Errorf("failed to build pull request request: %w", err) } p.applyAuthHeaders(req) - resp, err := p.httpClient.Do(req) + resp, err := networkgitea.Do(p.httpClient, req) if err != nil { return nil, fmt.Errorf("failed to fetch pull request: %w", err) } diff --git a/network/network_status.md b/network/network_status.md index 8a237d32..10567bc7 100644 --- a/network/network_status.md +++ b/network/network_status.md @@ -10,10 +10,20 @@ Latest milestone batch note (MF-051, MF-059, MF-073, MF-074, MF-076, MF-083, MF- | payment.CancelScheduledChangesByID | added | [CancelScheduledChangesByID](../internal/license/payment/subscription.go#L295) | | api.CreateSubscription | updated | [CreateSubscription](../internal/api/subscriptions_handler.go#L129) | | api.CancelSubscription | updated | [CancelSubscription](../internal/api/subscriptions_handler.go#L287) | -| api.PreviewUpgrade | updated | [PreviewUpgrade](../internal/api/billing_actions_handler.go#L457) | | api.GetBillingStatus | updated | [GetBillingStatus](../internal/api/billing_actions_handler.go#L1311) | +| api.checkGitHubParentCommentAuthor | updated | [checkGitHubParentCommentAuthor](../internal/api/unified_processor_v2.go#L707) | +| api.checkBitbucketParentCommentAuthor | updated | [checkBitbucketParentCommentAuthor](../internal/api/unified_processor_v2.go#L776) | +| api.PreviewUpgrade | updated | [PreviewUpgrade](../internal/api/billing_actions_handler.go#L457) | + | api.GetCurrentSubscription | updated | [GetCurrentSubscription](../internal/api/subscriptions_handler.go#L620) | | api.ListUserSubscriptions | updated | [ListUserSubscriptions](../internal/api/subscriptions_handler.go#L773) | +| providerinputgitea.fetchLatestReview | updated | [fetchLatestReview](../internal/provider_input/gitea/gitea_provider.go#L664) | +| providersgitea.GetMergeRequestDetails | updated | [GetMergeRequestDetails](../internal/providers/gitea/gitea_provider.go#L116) | +| providersgitea.GetMergeRequestChanges | updated | [GetMergeRequestChanges](../internal/providers/gitea/gitea_provider.go#L182) | +| providersgitea.PostComment | updated | [PostComment](../internal/providers/gitea/gitea_provider.go#L301) | +| providersgitea.postInlineViaSession | updated | [postInlineViaSession](../internal/providers/gitea/gitea_provider.go#L381) | +| providersgitea.ensureSession | updated | [ensureSession](../internal/providers/gitea/gitea_provider.go#L446) | +| providersgitea.fetchPullRequest | updated | [fetchPullRequest](../internal/providers/gitea/gitea_provider.go#L556) | | payment.cancellationVerified | updated | [cancellationVerified](../internal/license/payment/subscription_service.go#L1029) | | payment.verifyCancellationWithRetry | updated | [verifyCancellationWithRetry](../internal/license/payment/subscription_service.go#L1046) | | payment.handleSubscriptionCharged | updated | [handleSubscriptionCharged](../internal/license/payment/webhook_handler.go#L530) | diff --git a/osv-scanner.toml b/osv-scanner.toml index f26ddec5..693375ad 100644 --- a/osv-scanner.toml +++ b/osv-scanner.toml @@ -1,7 +1 @@ -[[IgnoredVulns]] -id = "GO-2026-4771" -reason = "Upstream maintainer indicates this report is not exploitable in practice for pgx in this context; tracked at https://github.com/jackc/pgx/issues/2530" - -[[IgnoredVulns]] -id = "GO-2026-4772" -reason = "Companion pgx advisory covered by the same maintainer rationale; tracked at https://github.com/jackc/pgx/issues/2530" +# OSV ignore list is currently empty.