From ee7093da29d13b7250526fd816419365cfabd1d6 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 20:48:53 -0500 Subject: [PATCH 01/14] feat: add github review-pr command and refactor workflow --- .github/workflows/pull-request.yml | 64 +--------- cmd/main/main.go | 197 +++++++++++++++++++++++++++++ go.mod | 3 + go.sum | 8 ++ internal/github/client.go | 131 +++++++++++++++++++ 5 files changed, 343 insertions(+), 60 deletions(-) create mode 100644 internal/github/client.go diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 334078a..2384d25 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -40,24 +40,12 @@ jobs: echo "No miso configuration found, using defaults" fi - - name: Review changed files (PR) + - name: Review and Comment on PR if: github.event_name == 'pull_request' env: OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} - run: | - # Get the base branch for comparison - BASE_SHA="${{ github.event.pull_request.base.sha }}" - HEAD_SHA="${{ github.event.pull_request.head.sha }}" - - echo "Reviewing changes between $BASE_SHA and $HEAD_SHA" - - # Review the diff - ./miso diff "$BASE_SHA..$HEAD_SHA" > review-output.md - - # Save review output as artifact - echo "# 🍲 miso Code review" > pr-review.md - echo "" >> pr-review.md - cat review-output.md >> pr-review.md + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: ./miso github review-pr - name: Review recent changes (Push) if: github.event_name == 'push' @@ -86,54 +74,10 @@ jobs: echo "" >> push-review.md cat review-output.md >> push-review.md - - name: Comment on PR - if: github.event_name == 'pull_request' - uses: actions/github-script@v7 - with: - script: | - const fs = require('fs'); - - // Read the review output - let reviewContent = ''; - try { - reviewContent = fs.readFileSync('pr-review.md', 'utf8'); - } catch (error) { - reviewContent = '# 🍲 miso Code review\n\nNo review content generated.'; - } - - // Find existing review comment - const comments = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - }); - - const botComment = comments.data.find(comment => - comment.user.type === 'Bot' && - comment.body.includes('🍲 miso Code review') - ); - - if (botComment) { - // Update existing comment - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: botComment.id, - body: reviewContent - }); - } else { - // Create new comment - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: reviewContent - }); - } - name: Upload review artifacts uses: actions/upload-artifact@v4 - if: always() + if: always() && github.event_name == 'push' with: name: code-review-output path: | diff --git a/cmd/main/main.go b/cmd/main/main.go index 8eeb447..d3b639e 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "fmt" "log" "os" @@ -16,6 +17,7 @@ import ( "github.com/j0lvera/miso/internal/config" "github.com/j0lvera/miso/internal/diff" "github.com/j0lvera/miso/internal/git" + misoGithub "github.com/j0lvera/miso/internal/github" "github.com/j0lvera/miso/internal/resolver" ) @@ -33,6 +35,7 @@ type CLI struct { Diff DiffCmd `cmd:"" help:"Review changes in a git diff"` ValidateConfig ValidateConfigCmd `cmd:"" help:"Validate configuration file"` TestPattern TestPatternCmd `cmd:"" help:"Test which patterns match a file"` + GitHub GitHubCmd `cmd:"" help:"GitHub integration commands"` Version VersionCmd `cmd:"" help:"Show version"` } @@ -302,6 +305,200 @@ type TestPatternCmd struct { Verbose bool `short:"v" help:"Show detailed matching info"` } +type GitHubCmd struct { + ReviewPR GitHubReviewPRCmd `cmd:"" help:"Review a PR and post a comment."` +} + +type GitHubReviewPRCmd struct { + PR int `short:"p" help:"Pull request number (auto-detected in GitHub Actions)."` + Base string `short:"b" help:"Base commit SHA (auto-detected in GitHub Actions)."` + Head string `short:"H" help:"Head commit SHA (auto-detected in GitHub Actions)."` + Verbose bool `short:"v" help:"Enable verbose output."` + Message string `short:"m" help:"Message to display while processing." default:"Analyzing PR..."` +} + +func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { + // Load configuration + parser := config.NewParser() + var cfg *config.Config + var err error + + if cli.Config != "" { + cfg, err = parser.LoadFile(cli.Config) + if err != nil { + return fmt.Errorf( + "failed to load config file %s: %w", cli.Config, err, + ) + } + if gr.Verbose { + fmt.Printf("Using config file: %s\n", cli.Config) + } + } else { + cfg, err = parser.Load() + if err != nil { + return fmt.Errorf("failed to load configuration: %w", err) + } + if gr.Verbose && len(cfg.Patterns) == 0 { + fmt.Println("Using default configuration (no config file found or config is empty)") + } + } + + ghClient, err := misoGithub.NewClient("") + if err != nil { + return fmt.Errorf("failed to initialize GitHub client: %w. Check GITHUB_TOKEN and GITHUB_REPOSITORY env vars", err) + } + + // Auto-detect PR info if not provided + base, head := gr.Base, gr.Head + prNumber := gr.PR + + if prNumber == 0 || base == "" || head == "" { + if event, err := ghClient.GetPRInfo(); err == nil { + if prNumber == 0 { + prNumber = event.PullRequest.Number + } + if base == "" { + base = event.PullRequest.Base.SHA + } + if head == "" { + head = event.PullRequest.Head.SHA + } + } + } + + if base == "" || head == "" { + return fmt.Errorf("could not determine base and head commits. Please specify --base and --head, or run in a GitHub Action context") + } + if prNumber == 0 { + return fmt.Errorf("could not determine pull request number. Please specify --pr, or run in a GitHub Action context") + } + + if gr.Verbose { + fmt.Printf("Reviewing PR #%d: %s..%s\n", prNumber, base, head) + } + + // Initialize git client + gitClient, err := git.NewGitClient() + if err != nil { + return fmt.Errorf("failed to initialize git client: %w", err) + } + + // Get changed files + files, err := gitClient.GetChangedFiles(base, head) + if err != nil { + return fmt.Errorf("failed to get changed files: %w", err) + } + + if len(files) == 0 { + fmt.Println("No files changed in the specified range.") + return nil + } + + if gr.Verbose { + fmt.Printf("Found %d changed files\n", len(files)) + } + + // Filter files that should be reviewed + res := resolver.NewResolver(cfg) + var reviewableFiles []string + for _, file := range files { + if res.ShouldReview(file) { + reviewableFiles = append(reviewableFiles, file) + } else if gr.Verbose { + fmt.Printf("Skipping %s (no matching patterns)\n", file) + } + } + + if len(reviewableFiles) == 0 { + fmt.Println("No files match review patterns.") + return nil + } + + // Initialize reviewer + reviewer, err := agents.NewCodeReviewer() + if err != nil { + return fmt.Errorf("failed to create reviewer: %w", err) + } + + // Capture review output + var reviewOutput bytes.Buffer + + // Review each changed file + totalTokens := 0 + formatter := diff.NewFormatter() + for _, file := range reviewableFiles { + // Get guides for this file + guides, err := res.GetDiffGuides(file) + if err != nil { + fmt.Printf("Error getting guides for file: %v\n", err) + continue + } + + if gr.Verbose { + fmt.Printf("Using diff guides: %v\n", guides) + } + + // Get the structured diff data + diffData, err := gitClient.GetFileDiffData(base, head, file) + if err != nil { + fmt.Printf("Error getting diff for file: %v\n", err) + continue + } + + // Create spinner + s := spinner.New(spinner.CharSets[spinnerCharSet], spinnerRefreshRate) + s.Suffix = " " + gr.Message + s.Start() + + // Perform diff review (reviewing only the changes) + result, err := reviewer.ReviewDiff(cfg, diffData, file) + + // Stop spinner + s.Stop() + + if err != nil { + fmt.Printf("Error reviewing file: %v\n", err) + continue + } + + // Format the output to include diffs + formattedContent := formatter.Format(result.Content) + + if formattedContent != "" { + reviewOutput.WriteString(fmt.Sprintf("
\n")) + reviewOutput.WriteString( + fmt.Sprintf( + "📝 Review for %s\n\n", file, + ), + ) + reviewOutput.WriteString(formattedContent) + reviewOutput.WriteString(fmt.Sprintf("\n
\n")) + } + + if result.TokensUsed > 0 { + totalTokens += result.TokensUsed + } + } + + // Post to GitHub + commentBody := fmt.Sprintf("# 🍲 miso Code review\n\n%s", reviewOutput.String()) + if err := ghClient.PostOrUpdateComment(prNumber, commentBody); err != nil { + return fmt.Errorf("failed to post comment to GitHub: %w", err) + } + fmt.Printf("✅ Successfully posted review to PR #%d\n", prNumber) + + // Summary for verbose mode + if gr.Verbose { + fmt.Printf("\n=== Summary ===\n") + fmt.Printf("Files reviewed: %d\n", len(reviewableFiles)) + if totalTokens > 0 { + fmt.Printf("Total tokens used: %d\n", totalTokens) + } + } + + return nil +} + func (d *DiffCmd) Run(cli *CLI) error { // Load configuration parser := config.NewParser() diff --git a/go.mod b/go.mod index 33a1355..aac34e8 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,10 @@ require ( github.com/briandowns/spinner v1.23.2 github.com/charmbracelet/glamour v0.10.0 github.com/go-git/go-git/v5 v5.16.2 + github.com/google/go-github/v57 v57.0.0 github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 github.com/tmc/langchaingo v0.1.13 + golang.org/x/oauth2 v0.21.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -37,6 +39,7 @@ require ( github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect github.com/go-git/go-billy/v5 v5.6.2 // indirect github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect + github.com/google/go-querystring v1.1.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/goph/emperror v0.17.2 // indirect github.com/gorilla/css v1.0.1 // indirect diff --git a/go.sum b/go.sum index acfd8c1..fdd463f 100644 --- a/go.sum +++ b/go.sum @@ -88,8 +88,13 @@ github.com/gofrs/uuid v3.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRx github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 h1:f+oWsMOmNPc8JmEHVZIycC7hBoQxHH9pNKQORJNozsQ= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8/go.mod h1:wcDNUvekVysuuOpQKo3191zZyTpiI6se1N1ULghS0sw= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/go-github/v57 v57.0.0 h1:L+Y3UPTY8ALM8x+TV0lg+IEBI+upibemtBD8Q9u7zHs= +github.com/google/go-github/v57 v57.0.0/go.mod h1:s0omdnye0hvK/ecLvpsGfJMiRt85PimQh4oygmLIxHw= +github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= +github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -233,6 +238,8 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY= golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E= +golang.org/x/oauth2 v0.21.0 h1:tsimM75w1tF/uws5rbeHzIWxEqElMehnc+iW793zsZs= +golang.org/x/oauth2 v0.21.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -268,6 +275,7 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= diff --git a/internal/github/client.go b/internal/github/client.go new file mode 100644 index 0000000..d0c40a2 --- /dev/null +++ b/internal/github/client.go @@ -0,0 +1,131 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "os" + "strings" + + "github.com/google/go-github/v57/github" + "golang.org/x/oauth2" +) + +type Client struct { + client *github.Client + owner string + repo string + ctx context.Context +} + +type PREvent struct { + PullRequest struct { + Number int `json:"number"` + Base struct { + SHA string `json:"sha"` + } `json:"base"` + Head struct { + SHA string `json:"sha"` + } `json:"head"` + } `json:"pull_request"` +} + +func NewClient(token string) (*Client, error) { + if token == "" { + token = os.Getenv("GITHUB_TOKEN") + if token == "" { + return nil, fmt.Errorf("GitHub token not provided and GITHUB_TOKEN not set") + } + } + + // Parse GITHUB_REPOSITORY env var (format: owner/repo) + repoEnv := os.Getenv("GITHUB_REPOSITORY") + if repoEnv == "" { + return nil, fmt.Errorf("GITHUB_REPOSITORY environment variable not set") + } + + parts := strings.Split(repoEnv, "/") + if len(parts) != 2 { + return nil, fmt.Errorf("invalid GITHUB_REPOSITORY format: %s", repoEnv) + } + + ctx := context.Background() + ts := oauth2.StaticTokenSource( + &oauth2.Token{AccessToken: token}, + ) + tc := oauth2.NewClient(ctx, ts) + + return &Client{ + client: github.NewClient(tc), + owner: parts[0], + repo: parts[1], + ctx: ctx, + }, nil +} + +func (c *Client) GetPRInfo() (*PREvent, error) { + eventPath := os.Getenv("GITHUB_EVENT_PATH") + if eventPath == "" { + return nil, fmt.Errorf("GITHUB_EVENT_PATH not set") + } + + data, err := os.ReadFile(eventPath) + if err != nil { + return nil, fmt.Errorf("failed to read event file: %w", err) + } + + var event PREvent + if err := json.Unmarshal(data, &event); err != nil { + return nil, fmt.Errorf("failed to parse event JSON: %w", err) + } + + return &event, nil +} + +func (c *Client) FindBotComment(prNumber int, identifier string) (*github.IssueComment, error) { + opts := &github.IssueListCommentsOptions{ + ListOptions: github.ListOptions{PerPage: 100}, + } + + comments, _, err := c.client.Issues.ListComments( + c.ctx, c.owner, c.repo, prNumber, opts, + ) + if err != nil { + return nil, err + } + + for _, comment := range comments { + if comment.User.GetType() == "Bot" && + strings.Contains(comment.GetBody(), identifier) { + return comment, nil + } + } + + return nil, nil +} + +func (c *Client) PostOrUpdateComment(prNumber int, content string) error { + identifier := "🍲 miso Code review" + + // Find existing comment + existing, err := c.FindBotComment(prNumber, identifier) + if err != nil { + return fmt.Errorf("failed to find existing comment: %w", err) + } + + if existing != nil { + // Update existing comment + _, _, err = c.client.Issues.EditComment( + c.ctx, c.owner, c.repo, existing.GetID(), + &github.IssueComment{Body: &content}, + ) + return err + } + + // Create new comment + _, _, err = c.client.Issues.CreateComment( + c.ctx, c.owner, c.repo, prNumber, + &github.IssueComment{Body: &content}, + ) + return err +} From 76ba984357c24c24695f21b154462281cac703d0 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 20:53:34 -0500 Subject: [PATCH 02/14] fix: explicitly name github command to prevent kebab-case conversion --- cmd/main/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index d3b639e..1fba626 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -35,7 +35,7 @@ type CLI struct { Diff DiffCmd `cmd:"" help:"Review changes in a git diff"` ValidateConfig ValidateConfigCmd `cmd:"" help:"Validate configuration file"` TestPattern TestPatternCmd `cmd:"" help:"Test which patterns match a file"` - GitHub GitHubCmd `cmd:"" help:"GitHub integration commands"` + GitHub GitHubCmd `cmd:"" name:"github" help:"GitHub integration commands"` Version VersionCmd `cmd:"" help:"Show version"` } From 4abef30087be523c4b79d5b2c9d1eb6ff75552cf Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:00:10 -0500 Subject: [PATCH 03/14] refactor: extract config loading to a shared function and add comment for pr cmd --- cmd/main/main.go | 128 ++++++++++++++++------------------------------- 1 file changed, 43 insertions(+), 85 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index 1fba626..efbfef5 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -108,25 +108,9 @@ func (vc *ValidateConfigCmd) Run(cli *CLI) error { } func (tp *TestPatternCmd) Run(cli *CLI) error { - parser := config.NewParser() - var cfg *config.Config - var err error - - if cli.Config != "" { - cfg, err = parser.LoadFile(cli.Config) - if err != nil { - return fmt.Errorf( - "failed to load config file %s: %w", cli.Config, err, - ) - } - } else { - cfg, err = parser.Load() - if err != nil { - return fmt.Errorf("failed to load configuration: %w", err) - } - if len(cfg.Patterns) == 0 { - fmt.Println("Using default configuration (no config file found or config is empty)") - } + cfg, err := loadConfig(cli.Config, tp.Verbose) + if err != nil { + return err } // Create resolver and test the file @@ -171,28 +155,9 @@ func (tp *TestPatternCmd) Run(cli *CLI) error { func (r *ReviewCmd) Run(cli *CLI) error { // Load configuration - parser := config.NewParser() - var cfg *config.Config - var err error - - if cli.Config != "" { - cfg, err = parser.LoadFile(cli.Config) - if err != nil { - return fmt.Errorf( - "failed to load config file %s: %w", cli.Config, err, - ) - } - if r.Verbose { - fmt.Printf("Using config file: %s\n", cli.Config) - } - } else { - cfg, err = parser.Load() - if err != nil { - return fmt.Errorf("failed to load configuration: %w", err) - } - if r.Verbose && len(cfg.Patterns) == 0 { - fmt.Println("Using default configuration (no config file found or config is empty)") - } + cfg, err := loadConfig(cli.Config, r.Verbose) + if err != nil { + return err } // Check if file should be reviewed @@ -309,6 +274,10 @@ type GitHubCmd struct { ReviewPR GitHubReviewPRCmd `cmd:"" help:"Review a PR and post a comment."` } +// GitHubReviewPRCmd reviews a pull request. +// The fields PR, Base, and Head are intentionally not marked as 'required' +// because they are designed to be auto-detected from the GitHub Actions environment. +// The validation logic is handled within the Run method after attempting auto-detection. type GitHubReviewPRCmd struct { PR int `short:"p" help:"Pull request number (auto-detected in GitHub Actions)."` Base string `short:"b" help:"Base commit SHA (auto-detected in GitHub Actions)."` @@ -319,28 +288,9 @@ type GitHubReviewPRCmd struct { func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { // Load configuration - parser := config.NewParser() - var cfg *config.Config - var err error - - if cli.Config != "" { - cfg, err = parser.LoadFile(cli.Config) - if err != nil { - return fmt.Errorf( - "failed to load config file %s: %w", cli.Config, err, - ) - } - if gr.Verbose { - fmt.Printf("Using config file: %s\n", cli.Config) - } - } else { - cfg, err = parser.Load() - if err != nil { - return fmt.Errorf("failed to load configuration: %w", err) - } - if gr.Verbose && len(cfg.Patterns) == 0 { - fmt.Println("Using default configuration (no config file found or config is empty)") - } + cfg, err := loadConfig(cli.Config, gr.Verbose) + if err != nil { + return err } ghClient, err := misoGithub.NewClient("") @@ -501,28 +451,9 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { func (d *DiffCmd) Run(cli *CLI) error { // Load configuration - parser := config.NewParser() - var cfg *config.Config - var err error - - if cli.Config != "" { - cfg, err = parser.LoadFile(cli.Config) - if err != nil { - return fmt.Errorf( - "failed to load config file %s: %w", cli.Config, err, - ) - } - if d.Verbose { - fmt.Printf("Using config file: %s\n", cli.Config) - } - } else { - cfg, err = parser.Load() - if err != nil { - return fmt.Errorf("failed to load configuration: %w", err) - } - if d.Verbose && len(cfg.Patterns) == 0 { - fmt.Println("Using default configuration (no config file found or config is empty)") - } + cfg, err := loadConfig(cli.Config, d.Verbose) + if err != nil { + return err } // Initialize git client @@ -880,6 +811,33 @@ func renderRichOutput(content string) (string, error) { return rendered, nil } +func loadConfig(configPath string, verbose bool) (*config.Config, error) { + parser := config.NewParser() + var cfg *config.Config + var err error + + if configPath != "" { + cfg, err = parser.LoadFile(configPath) + if err != nil { + return nil, fmt.Errorf( + "failed to load config file %s: %w", configPath, err, + ) + } + if verbose { + fmt.Printf("Using config file: %s\n", configPath) + } + } else { + cfg, err = parser.Load() + if err != nil { + return nil, fmt.Errorf("failed to load configuration: %w", err) + } + if verbose && len(cfg.Patterns) == 0 { + fmt.Println("Using default configuration (no config file found or config is empty)") + } + } + return cfg, nil +} + func main() { var cli CLI ctx := kong.Parse( From c11af2c0b861ba8dbad17d22fdabbfd6f6b9f629 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:01:45 -0500 Subject: [PATCH 04/14] fix: standardize github client initialization error message --- cmd/main/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index efbfef5..4228621 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -295,7 +295,7 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { ghClient, err := misoGithub.NewClient("") if err != nil { - return fmt.Errorf("failed to initialize GitHub client: %w. Check GITHUB_TOKEN and GITHUB_REPOSITORY env vars", err) + return fmt.Errorf("failed to initialize GitHub client (check GITHUB_TOKEN and GITHUB_REPOSITORY env vars): %w", err) } // Auto-detect PR info if not provided From 0af64c904bb2ec3e6e4f55866815e79e417a4ece Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:03:12 -0500 Subject: [PATCH 05/14] feat: add input validation for github review-pr command --- cmd/main/main.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/cmd/main/main.go b/cmd/main/main.go index 4228621..e5665d4 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -286,7 +286,30 @@ type GitHubReviewPRCmd struct { Message string `short:"m" help:"Message to display while processing." default:"Analyzing PR..."` } +func isValidSHA(sha string) bool { + // A git SHA is typically 40 hex characters, but can be shorter (e.g. 7). + // We'll check for 4-40 hex characters. + match, _ := regexp.MatchString(`^[a-f0-9]{4,40}$`, sha) + return match +} + +func (gr *GitHubReviewPRCmd) validate() error { + if gr.PR < 0 { + return fmt.Errorf("invalid PR number: %d", gr.PR) + } + if gr.Base != "" && !isValidSHA(gr.Base) { + return fmt.Errorf("invalid base SHA: %s", gr.Base) + } + if gr.Head != "" && !isValidSHA(gr.Head) { + return fmt.Errorf("invalid head SHA: %s", gr.Head) + } + return nil +} + func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { + if err := gr.validate(); err != nil { + return err + } // Load configuration cfg, err := loadConfig(cli.Config, gr.Verbose) if err != nil { From 43527cbffa9ebd9421657065f79f3d00420667eb Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:03:55 -0500 Subject: [PATCH 06/14] fix: add github api rate limit error handling --- internal/github/client.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/github/client.go b/internal/github/client.go index d0c40a2..66c416a 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -91,6 +91,9 @@ func (c *Client) FindBotComment(prNumber int, identifier string) (*github.IssueC c.ctx, c.owner, c.repo, prNumber, opts, ) if err != nil { + if _, ok := err.(*github.RateLimitError); ok { + return nil, fmt.Errorf("GitHub API rate limit exceeded, please try again later") + } return nil, err } @@ -119,6 +122,9 @@ func (c *Client) PostOrUpdateComment(prNumber int, content string) error { c.ctx, c.owner, c.repo, existing.GetID(), &github.IssueComment{Body: &content}, ) + if _, ok := err.(*github.RateLimitError); ok { + return fmt.Errorf("GitHub API rate limit exceeded, please try again later") + } return err } @@ -127,5 +133,8 @@ func (c *Client) PostOrUpdateComment(prNumber int, content string) error { c.ctx, c.owner, c.repo, prNumber, &github.IssueComment{Body: &content}, ) + if _, ok := err.(*github.RateLimitError); ok { + return fmt.Errorf("GitHub API rate limit exceeded, please try again later") + } return err } From 9e97bbbfdc4fa1c28dfecd638aefb28106491167 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:05:27 -0500 Subject: [PATCH 07/14] fix: paginate github api calls for finding bot comments --- internal/github/client.go | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index 66c416a..b28c1df 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -87,21 +87,28 @@ func (c *Client) FindBotComment(prNumber int, identifier string) (*github.IssueC ListOptions: github.ListOptions{PerPage: 100}, } - comments, _, err := c.client.Issues.ListComments( - c.ctx, c.owner, c.repo, prNumber, opts, - ) - if err != nil { - if _, ok := err.(*github.RateLimitError); ok { - return nil, fmt.Errorf("GitHub API rate limit exceeded, please try again later") + for { + comments, resp, err := c.client.Issues.ListComments( + c.ctx, c.owner, c.repo, prNumber, opts, + ) + if err != nil { + if _, ok := err.(*github.RateLimitError); ok { + return nil, fmt.Errorf("GitHub API rate limit exceeded, please try again later") + } + return nil, err + } + + for _, comment := range comments { + if comment.User != nil && comment.User.GetType() == "Bot" && + strings.Contains(comment.GetBody(), identifier) { + return comment, nil + } } - return nil, err - } - for _, comment := range comments { - if comment.User.GetType() == "Bot" && - strings.Contains(comment.GetBody(), identifier) { - return comment, nil + if resp.NextPage == 0 { + break } + opts.Page = resp.NextPage } return nil, nil From cf60f1f7599503d5af2566ddb6d87d19e3a068fc Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:08:09 -0500 Subject: [PATCH 08/14] fix: add pr number to github comment post error message --- cmd/main/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index e5665d4..eebbdf4 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -456,7 +456,7 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { // Post to GitHub commentBody := fmt.Sprintf("# 🍲 miso Code review\n\n%s", reviewOutput.String()) if err := ghClient.PostOrUpdateComment(prNumber, commentBody); err != nil { - return fmt.Errorf("failed to post comment to GitHub: %w", err) + return fmt.Errorf("failed to post comment to GitHub (PR #%d): %w", prNumber, err) } fmt.Printf("✅ Successfully posted review to PR #%d\n", prNumber) From 9b9fceadb04019b4e6c66567414920915c6ad0c5 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:08:45 -0500 Subject: [PATCH 09/14] refactor: improve verbose output for github review-pr command --- cmd/main/main.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index eebbdf4..e1a4e91 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -462,11 +462,8 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { // Summary for verbose mode if gr.Verbose { - fmt.Printf("\n=== Summary ===\n") - fmt.Printf("Files reviewed: %d\n", len(reviewableFiles)) - if totalTokens > 0 { - fmt.Printf("Total tokens used: %d\n", totalTokens) - } + log.Printf("Review completed: Files=%d, Tokens=%d, PR=#%d\n", + len(reviewableFiles), totalTokens, prNumber) } return nil From 629862a9c1b2ce6eb94daa037c14c525993d7fd0 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:10:46 -0500 Subject: [PATCH 10/14] feat: add context timeout for github operations --- cmd/main/main.go | 5 ++++- internal/github/client.go | 14 ++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index e1a4e91..dcb6a87 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "context" "fmt" "log" "os" @@ -455,7 +456,9 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { // Post to GitHub commentBody := fmt.Sprintf("# 🍲 miso Code review\n\n%s", reviewOutput.String()) - if err := ghClient.PostOrUpdateComment(prNumber, commentBody); err != nil { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if err := ghClient.PostOrUpdateComment(ctx, prNumber, commentBody); err != nil { return fmt.Errorf("failed to post comment to GitHub (PR #%d): %w", prNumber, err) } fmt.Printf("✅ Successfully posted review to PR #%d\n", prNumber) diff --git a/internal/github/client.go b/internal/github/client.go index b28c1df..8de0815 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -15,7 +15,6 @@ type Client struct { client *github.Client owner string repo string - ctx context.Context } type PREvent struct { @@ -59,7 +58,6 @@ func NewClient(token string) (*Client, error) { client: github.NewClient(tc), owner: parts[0], repo: parts[1], - ctx: ctx, }, nil } @@ -82,14 +80,14 @@ func (c *Client) GetPRInfo() (*PREvent, error) { return &event, nil } -func (c *Client) FindBotComment(prNumber int, identifier string) (*github.IssueComment, error) { +func (c *Client) FindBotComment(ctx context.Context, prNumber int, identifier string) (*github.IssueComment, error) { opts := &github.IssueListCommentsOptions{ ListOptions: github.ListOptions{PerPage: 100}, } for { comments, resp, err := c.client.Issues.ListComments( - c.ctx, c.owner, c.repo, prNumber, opts, + ctx, c.owner, c.repo, prNumber, opts, ) if err != nil { if _, ok := err.(*github.RateLimitError); ok { @@ -114,11 +112,11 @@ func (c *Client) FindBotComment(prNumber int, identifier string) (*github.IssueC return nil, nil } -func (c *Client) PostOrUpdateComment(prNumber int, content string) error { +func (c *Client) PostOrUpdateComment(ctx context.Context, prNumber int, content string) error { identifier := "🍲 miso Code review" // Find existing comment - existing, err := c.FindBotComment(prNumber, identifier) + existing, err := c.FindBotComment(ctx, prNumber, identifier) if err != nil { return fmt.Errorf("failed to find existing comment: %w", err) } @@ -126,7 +124,7 @@ func (c *Client) PostOrUpdateComment(prNumber int, content string) error { if existing != nil { // Update existing comment _, _, err = c.client.Issues.EditComment( - c.ctx, c.owner, c.repo, existing.GetID(), + ctx, c.owner, c.repo, existing.GetID(), &github.IssueComment{Body: &content}, ) if _, ok := err.(*github.RateLimitError); ok { @@ -137,7 +135,7 @@ func (c *Client) PostOrUpdateComment(prNumber int, content string) error { // Create new comment _, _, err = c.client.Issues.CreateComment( - c.ctx, c.owner, c.repo, prNumber, + ctx, c.owner, c.repo, prNumber, &github.IssueComment{Body: &content}, ) if _, ok := err.(*github.RateLimitError); ok { From 1d6de4e287855ae57373f69a351bcc5b43f35cf7 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:12:49 -0500 Subject: [PATCH 11/14] feat: implement cleanup of old bot comments on pull requests --- cmd/main/main.go | 10 ++++++ internal/github/client.go | 66 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/cmd/main/main.go b/cmd/main/main.go index dcb6a87..78e4bba 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -463,6 +463,16 @@ func (gr *GitHubReviewPRCmd) Run(cli *CLI) error { } fmt.Printf("✅ Successfully posted review to PR #%d\n", prNumber) + // Clean up old comments + cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cleanupCancel() + if err := ghClient.CleanupOldComments(cleanupCtx, prNumber); err != nil { + // This is not a fatal error, so just log it. + if gr.Verbose { + log.Printf("Failed to clean up old comments: %v", err) + } + } + // Summary for verbose mode if gr.Verbose { log.Printf("Review completed: Files=%d, Tokens=%d, PR=#%d\n", diff --git a/internal/github/client.go b/internal/github/client.go index 8de0815..ad83ac5 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -4,13 +4,17 @@ import ( "context" "encoding/json" "fmt" + "log" "os" + "sort" "strings" "github.com/google/go-github/v57/github" "golang.org/x/oauth2" ) +const BotCommentIdentifier = "🍲 miso Code review" + type Client struct { client *github.Client owner string @@ -80,7 +84,9 @@ func (c *Client) GetPRInfo() (*PREvent, error) { return &event, nil } -func (c *Client) FindBotComment(ctx context.Context, prNumber int, identifier string) (*github.IssueComment, error) { +// findAllBotComments is a helper to find all comments by the bot matching an identifier. +func (c *Client) findAllBotComments(ctx context.Context, prNumber int, identifier string) ([]*github.IssueComment, error) { + var botComments []*github.IssueComment opts := &github.IssueListCommentsOptions{ ListOptions: github.ListOptions{PerPage: 100}, } @@ -99,7 +105,7 @@ func (c *Client) FindBotComment(ctx context.Context, prNumber int, identifier st for _, comment := range comments { if comment.User != nil && comment.User.GetType() == "Bot" && strings.Contains(comment.GetBody(), identifier) { - return comment, nil + botComments = append(botComments, comment) } } @@ -109,11 +115,29 @@ func (c *Client) FindBotComment(ctx context.Context, prNumber int, identifier st opts.Page = resp.NextPage } - return nil, nil + return botComments, nil +} + +func (c *Client) FindBotComment(ctx context.Context, prNumber int, identifier string) (*github.IssueComment, error) { + allComments, err := c.findAllBotComments(ctx, prNumber, identifier) + if err != nil { + return nil, err + } + + if len(allComments) == 0 { + return nil, nil + } + + // Sort by creation time to find the most recent one + sort.Slice(allComments, func(i, j int) bool { + return allComments[i].GetCreatedAt().After(allComments[j].GetCreatedAt().Time) + }) + + return allComments[0], nil } func (c *Client) PostOrUpdateComment(ctx context.Context, prNumber int, content string) error { - identifier := "🍲 miso Code review" + identifier := BotCommentIdentifier // Find existing comment existing, err := c.FindBotComment(ctx, prNumber, identifier) @@ -143,3 +167,37 @@ func (c *Client) PostOrUpdateComment(ctx context.Context, prNumber int, content } return err } + +// CleanupOldComments finds all comments posted by the bot with the specific identifier +// and deletes all but the most recent one. +func (c *Client) CleanupOldComments(ctx context.Context, prNumber int) error { + identifier := BotCommentIdentifier + allBotComments, err := c.findAllBotComments(ctx, prNumber, identifier) + if err != nil { + return fmt.Errorf("failed to find bot comments for cleanup: %w", err) + } + + if len(allBotComments) <= 1 { + return nil // Nothing to clean up + } + + // Sort comments by creation time, newest first + sort.Slice(allBotComments, func(i, j int) bool { + return allBotComments[i].GetCreatedAt().After(allBotComments[j].GetCreatedAt().Time) + }) + + // Keep the first one (newest), delete the rest + commentsToDelete := allBotComments[1:] + if len(commentsToDelete) > 0 { + log.Printf("Cleaning up %d old bot comments for PR #%d", len(commentsToDelete), prNumber) + } + for _, comment := range commentsToDelete { + _, err := c.client.Issues.DeleteComment(ctx, c.owner, c.repo, comment.GetID()) + if err != nil { + // Log error but continue trying to delete others + log.Printf("failed to delete old bot comment #%d: %v", comment.GetID(), err) + } + } + + return nil +} From a46815f82fc9ad50d157af3c7415776bf289c703 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:18:54 -0500 Subject: [PATCH 12/14] fix: handle github api rate limit during comment cleanup --- internal/github/client.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/github/client.go b/internal/github/client.go index ad83ac5..8520742 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -194,6 +194,11 @@ func (c *Client) CleanupOldComments(ctx context.Context, prNumber int) error { for _, comment := range commentsToDelete { _, err := c.client.Issues.DeleteComment(ctx, c.owner, c.repo, comment.GetID()) if err != nil { + if _, ok := err.(*github.RateLimitError); ok { + // Rate limit hit, stop trying to delete more comments + log.Printf("GitHub API rate limit exceeded while cleaning up comments, stopping cleanup") + return fmt.Errorf("GitHub API rate limit exceeded during cleanup") + } // Log error but continue trying to delete others log.Printf("failed to delete old bot comment #%d: %v", comment.GetID(), err) } From b1be7f62bcb3337a6d7125d0812df28dcd6fd291 Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:19:39 -0500 Subject: [PATCH 13/14] refactor: extract comment sorting logic to avoid duplication --- internal/github/client.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index 8520742..0a5113f 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -118,6 +118,13 @@ func (c *Client) findAllBotComments(ctx context.Context, prNumber int, identifie return botComments, nil } +// sortCommentsByCreatedDesc sorts comments by creation time, newest first. +func sortCommentsByCreatedDesc(comments []*github.IssueComment) { + sort.Slice(comments, func(i, j int) bool { + return comments[i].GetCreatedAt().After(comments[j].GetCreatedAt().Time) + }) +} + func (c *Client) FindBotComment(ctx context.Context, prNumber int, identifier string) (*github.IssueComment, error) { allComments, err := c.findAllBotComments(ctx, prNumber, identifier) if err != nil { @@ -129,9 +136,7 @@ func (c *Client) FindBotComment(ctx context.Context, prNumber int, identifier st } // Sort by creation time to find the most recent one - sort.Slice(allComments, func(i, j int) bool { - return allComments[i].GetCreatedAt().After(allComments[j].GetCreatedAt().Time) - }) + sortCommentsByCreatedDesc(allComments) return allComments[0], nil } @@ -182,9 +187,7 @@ func (c *Client) CleanupOldComments(ctx context.Context, prNumber int) error { } // Sort comments by creation time, newest first - sort.Slice(allBotComments, func(i, j int) bool { - return allBotComments[i].GetCreatedAt().After(allBotComments[j].GetCreatedAt().Time) - }) + sortCommentsByCreatedDesc(allBotComments) // Keep the first one (newest), delete the rest commentsToDelete := allBotComments[1:] From 523b43557702e0cdedc0a932da1cd928f72ebe8d Mon Sep 17 00:00:00 2001 From: Juan Olvera Date: Mon, 21 Jul 2025 21:21:41 -0500 Subject: [PATCH 14/14] ci: add concurrency control to pull request workflow --- .github/workflows/pull-request.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 2384d25..3f680a8 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -6,6 +6,11 @@ on: push: branches: [ main ] +# Prevent multiple workflow runs for the same PR +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: code-review: runs-on: ubuntu-latest