-
Notifications
You must be signed in to change notification settings - Fork 8
Gitea Integrations issues fixes #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a2f112f
5033e08
d79a5c3
1ba7b02
7bfe07a
f98e65d
d25c0b2
4300869
ad7270c
d4e352e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this confidence value? where it is used and why it is 0.5?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gitea doesn't have proper webhooks. whenever new comment posted, it doesn't have any data in the webhook. So, there is a 50% probability whether the reply is needed or not based on fetching the recent comment posted in the PR. So, it is set up to 0.5 to identify the confidence level of the comment. |
||
| 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did this change to networkGitHub? What is the difference?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might not be related to Gitea issues, but this change has to be done due to rule 2 in Copilot instructions and security audits.
|
||
| 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,16 +823,16 @@ 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 | ||
| } | ||
|
|
||
| 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,32 +1598,36 @@ 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, "/") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the difference here from previous patchURL construction?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old version was directly fetching the patch URL in the UI, which was added to send in the webhook. As I checked a few cases, the path URL was not sent in the webhook from the Gitea side. |
||
| 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 == "" { | ||
| return nil, fmt.Errorf("missing gitea patch URL") | ||
| } | ||
|
|
||
| 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) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.