monitor Linter CI check and comment @Copilot fixs it#64
monitor Linter CI check and comment @Copilot fixs it#64juyixi wants to merge 1 commit intocasdoor:masterfrom
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
Adds infrastructure to monitor failing CI/linter check runs via GitHub webhooks, persist failure/attempt state, and notify/tag Copilot to help fix issues automatically.
Changes:
- Add
check_runwebhook handling to detect failed linter-related checks and comment on PRs tagging Copilot. - Persist PR check failure state (including fix-attempt counts) in a new
PrChecktable and expose read APIs for it. - Add endpoints to query PR check records and register them in the router.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
util/check_api.go |
New GitHub Checks helpers: list check runs, identify linter checks, format failure details, and notify/request Copilot. |
controllers/webhook.go |
Handle check_run events; on linter failures, store/update PrCheck and comment/tag Copilot; also request Copilot review on PR open. |
object/pr_check.go |
New PrCheck model + persistence helpers + attempt gating logic. |
object/adapter.go |
Auto-migrate new PrCheck table. |
controllers/pr_check.go |
New API handlers to fetch PR check records. |
routers/router.go |
Register new PR-check query endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Only process failed checks. | ||
| if checkRun.GetConclusion() != "failure" && checkRun.GetConclusion() != "cancelled" { |
There was a problem hiding this comment.
Treating cancelled as a failure will burn fix attempts and spam PRs for runs that were cancelled due to superseding commits or manual cancels (there’s nothing to “fix”). Consider triggering fix attempts only for actionable conclusions (typically failure, maybe timed_out), and ignore cancelled/skipped/stale.
| // Only process failed checks. | |
| if checkRun.GetConclusion() != "failure" && checkRun.GetConclusion() != "cancelled" { | |
| // Only process actionable failed checks. | |
| conclusion := checkRun.GetConclusion() | |
| if conclusion != "failure" && conclusion != "timed_out" { |
| // Only process failed checks. | ||
| if checkRun.GetConclusion() != "failure" && checkRun.GetConclusion() != "cancelled" { | ||
| return false | ||
| } |
There was a problem hiding this comment.
The PrCheck.IsFixed flag is never set to true because this handler returns early for non-failures and doesn’t process successful completions. This makes IsFixed ineffective and also prevents resetting attempts when a check is later fixed. Consider handling completed+success (and possibly neutral/skipped) to update the existing PrCheck record as fixed (and optionally reset attempts).
| // ShouldAttemptFix checks if we should attempt to fix this check (max attempts configurable). | ||
| func ShouldAttemptFix(org string, repo string, prNumber int, checkName string) bool { | ||
| prCheck := GetPrCheck(org, repo, prNumber, checkName) | ||
| // First time, should attempt. | ||
| if prCheck == nil { | ||
| return true | ||
| } | ||
| return prCheck.FixAttempts < util.MaxFixAttempts && !prCheck.IsFixed | ||
| } |
There was a problem hiding this comment.
ShouldAttemptFix gates on !prCheck.IsFixed, but nothing in this PR ever marks IsFixed true, so this condition is currently ineffective and attempts never reset across fix cycles. Consider adding a MarkPrCheckFixed (and possibly ResetFixAttempts) update path and invoking it when the check run completes successfully.
| // We ignore errors here as the comment is the primary notification. | ||
| _ = RequestReviewers(owner, repo, prNumber, []string{CopilotUsername}) |
There was a problem hiding this comment.
RequestCopilotReview says it ignores errors from requesting reviewers, but util.RequestReviewers panics on API errors (see util/github_api.go). That means this line can crash the process in common cases like 403/422 when Copilot isn’t a valid reviewer. Consider changing RequestReviewers/Comment to return errors instead of panicking, or wrap this call with proper error handling/recover and log the failure.
| // We ignore errors here as the comment is the primary notification. | |
| _ = RequestReviewers(owner, repo, prNumber, []string{CopilotUsername}) | |
| // We ignore errors here as the comment is the primary notification, and | |
| // also guard against panics from the underlying API call. | |
| func() { | |
| defer func() { | |
| if r := recover(); r != nil { | |
| fmt.Printf("warning: failed to request Copilot review for PR #%d: %v\n", prNumber, r) | |
| } | |
| }() | |
| _ = RequestReviewers(owner, repo, prNumber, []string{CopilotUsername}) | |
| }() |
| result, _, err := client.Checks.ListCheckRunsForRef(context.Background(), owner, repo, ref, opts) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return result.CheckRuns, nil |
There was a problem hiding this comment.
GetPRCheckRuns fetches only the first page (PerPage=100) and does not paginate, so it can silently miss check runs on refs with >100 runs. Consider iterating pages using ListOptions.Page and resp.NextPage until exhausted.
| result, _, err := client.Checks.ListCheckRunsForRef(context.Background(), owner, repo, ref, opts) | |
| if err != nil { | |
| return nil, err | |
| } | |
| return result.CheckRuns, nil | |
| var allCheckRuns []*github.CheckRun | |
| ctx := context.Background() | |
| for { | |
| result, resp, err := client.Checks.ListCheckRunsForRef(ctx, owner, repo, ref, opts) | |
| if err != nil { | |
| return nil, err | |
| } | |
| allCheckRuns = append(allCheckRuns, result.CheckRuns...) | |
| if resp == nil || resp.NextPage == 0 { | |
| break | |
| } | |
| opts.Page = resp.NextPage | |
| } | |
| return allCheckRuns, nil |
| // Request automatic code review from copilot for human-created PRs. | ||
| go util.RequestCopilotReview(owner, repo, pullRequestEvent.GetNumber()) | ||
| } |
There was a problem hiding this comment.
This is labeled “human-created PRs”, but there’s no check to exclude bot PRs (e.g. dependabot/github-actions) or Copilot itself. That can create noisy comments and potential feedback loops; consider filtering by pullRequestEvent.PullRequest.User.GetType() / Sender.GetType() (skip "Bot") or logins ending with [bot], and skip when sender matches the configured Copilot username.
fixs #62