From d6a6d66f55a5018d6aeac903ae1766ca64171797 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:00:47 -0500 Subject: [PATCH 01/37] Diff commenter --- .github/workflows/auto-casdiff-comment.yaml | 36 ++++ .gitignore | 1 + cmd/commentprcasdiff/README.md | 73 +++++++ cmd/commentprcasdiff/casdiff_runner.go | 76 +++++++ cmd/commentprcasdiff/comment_poster.go | 111 ++++++++++ cmd/commentprcasdiff/main.go | 150 +++++++++++++ cmd/commentprcasdiff/module_finder.go | 62 ++++++ cmd/commentprcasdiff/state_analyzer.go | 222 ++++++++++++++++++++ cmd/commentprcasdiff/state_analyzer_test.go | 119 +++++++++++ cmd/release/main.go | 6 +- 10 files changed, 853 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/auto-casdiff-comment.yaml create mode 100644 cmd/commentprcasdiff/README.md create mode 100644 cmd/commentprcasdiff/casdiff_runner.go create mode 100644 cmd/commentprcasdiff/comment_poster.go create mode 100644 cmd/commentprcasdiff/main.go create mode 100644 cmd/commentprcasdiff/module_finder.go create mode 100644 cmd/commentprcasdiff/state_analyzer.go create mode 100644 cmd/commentprcasdiff/state_analyzer_test.go diff --git a/.github/workflows/auto-casdiff-comment.yaml b/.github/workflows/auto-casdiff-comment.yaml new file mode 100644 index 00000000..7bb887f7 --- /dev/null +++ b/.github/workflows/auto-casdiff-comment.yaml @@ -0,0 +1,36 @@ +name: Auto-comment PR with casdiff + +on: + pull_request: + types: [opened, synchronize] + branches: + - fetch-modules + +permissions: + contents: read + pull-requests: write + +jobs: + comment-changes: + runs-on: ubuntu-latest + if: github.repository == 'bufbuild/modules' + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + fetch-depth: 0 # Need full history to compare branches + + - name: Install Go + uses: actions/setup-go@v6 + with: + go-version: 1.24.x + check-latest: true + cache: true + + - name: Run commentprcasdiff + run: go run ./cmd/commentprcasdiff + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + BASE_REF: ${{ github.event.pull_request.base.sha }} + HEAD_REF: ${{ github.event.pull_request.head.sha }} diff --git a/.gitignore b/.gitignore index a5fc3962..dfcdbb86 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /.tmp/ +/.claude/ diff --git a/cmd/commentprcasdiff/README.md b/cmd/commentprcasdiff/README.md new file mode 100644 index 00000000..a8ac5f12 --- /dev/null +++ b/cmd/commentprcasdiff/README.md @@ -0,0 +1,73 @@ +# commentprcasdiff + +Automates posting CAS diff comments on PRs when module digest changes are detected. + +## What it does + +This tool runs in GitHub Actions when a PR is created against the `fetch-modules` branch. It: + +1. Identifies which `state.json` files changed in the PR +2. Compares old and new state to find digest transitions (hash changes) +3. Runs `casdiff` for each transition to show what changed +4. Posts the output as inline review comments at the line where each new digest appears + +## How it works + +### State Analysis + +Each module's `state.json` file is append-only and contains references with their content digests: + +```json +{ + "references": [ + {"name": "v1.0.0", "digest": "aaa..."}, + {"name": "v1.1.0", "digest": "bbb..."} + ] +} +``` + +The tool: +- Reads the full JSON from both the base branch (`main`) and head branch (`fetch-modules`) +- Identifies newly appended references +- Detects when the digest changes between consecutive references +- For each digest change, runs: `casdiff --format=markdown` + +### Comment Posting + +Comments are posted as PR review comments on the specific line where the new digest first appears in the diff, similar to manual code review comments. + +Example: If digest changes from `aaa` to `bbb` at reference `v1.1.0`, a comment is posted at the line containing `"digest": "bbb"` in the state.json diff. + +## Local Testing + +To test the command locally: + +```bash +# Set required environment variables +export PR_NUMBER=1234 +export BASE_REF=main +export HEAD_REF=fetch-modules +export GITHUB_TOKEN= + +# Run the command +go run ./cmd/commentprcasdiff +``` + +**Note:** The command expects to be run from the repository root and requires: +- Git repository with the specified refs +- GitHub CLI (`gh`) installed and authenticated +- Access to post PR comments via GitHub API + +## Architecture + +- **main.go**: Entry point, orchestrates the workflow +- **module_finder.go**: Finds changed `state.json` files using git diff +- **state_analyzer.go**: Compares JSON arrays to detect digest transitions +- **casdiff_runner.go**: Executes casdiff commands in parallel +- **comment_poster.go**: Posts review comments via GitHub API + +## Error Handling + +- If casdiff fails for a transition, the error is logged but doesn't stop the workflow +- Successful transitions still get commented +- All failures are summarized at the end diff --git a/cmd/commentprcasdiff/casdiff_runner.go b/cmd/commentprcasdiff/casdiff_runner.go new file mode 100644 index 00000000..d0d1dcec --- /dev/null +++ b/cmd/commentprcasdiff/casdiff_runner.go @@ -0,0 +1,76 @@ +// Copyright 2021-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "bytes" + "context" + "fmt" + "os/exec" + "sync" +) + +// CASDiffResult contains the result of running casdiff for a transition. +type CASDiffResult struct { + Transition StateTransition + Output string // Markdown output from casdiff + Error error +} + +// RunCASDiff executes casdiff command in the module directory. +func RunCASDiff(ctx context.Context, transition StateTransition) CASDiffResult { + result := CASDiffResult{ + Transition: transition, + } + + // Run casdiff in the module directory + cmd := exec.CommandContext( + ctx, + "go", "run", "../../cmd/casdiff", + transition.FromRef, + transition.ToRef, + "--format=markdown", + ) + cmd.Dir = transition.ModulePath + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + result.Error = fmt.Errorf("casdiff failed: %w (stderr: %s)", err, stderr.String()) + return result + } + + result.Output = stdout.String() + return result +} + +// RunCASDiffParallel runs multiple casdiff commands concurrently. +func RunCASDiffParallel(ctx context.Context, transitions []StateTransition) []CASDiffResult { + results := make([]CASDiffResult, len(transitions)) + var wg sync.WaitGroup + + for i, transition := range transitions { + wg.Add(1) + go func(index int, trans StateTransition) { + defer wg.Done() + results[index] = RunCASDiff(ctx, trans) + }(i, transition) + } + + wg.Wait() + return results +} diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go new file mode 100644 index 00000000..fb31d701 --- /dev/null +++ b/cmd/commentprcasdiff/comment_poster.go @@ -0,0 +1,111 @@ +// Copyright 2021-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" +) + +// PRReviewComment represents a comment to post on a specific line in a PR. +type PRReviewComment struct { + PRNumber string + FilePath string // File path in the PR (e.g., "modules/sync/bufbuild/protovalidate/state.json") + LineNumber int // Line number in the diff + Body string // Comment body (casdiff output) + CommitID string // Head commit SHA +} + +// PostReviewComments posts review comments to specific lines in the PR diff. +// Uses GitHub API (via gh CLI) to create inline comments. +func PostReviewComments(ctx context.Context, comments ...PRReviewComment) []error { + var errors []error + + // Get repository information + repo, err := getRepositoryInfo(ctx) + if err != nil { + for range comments { + errors = append(errors, fmt.Errorf("get repository info: %w", err)) + } + return errors + } + + // Post each comment + for _, comment := range comments { + if err := postSingleReviewComment(ctx, repo, comment); err != nil { + errors = append(errors, err) + } else { + errors = append(errors, nil) + } + } + + return errors +} + +// getRepositoryInfo gets the owner/repo from the current git repository. +func getRepositoryInfo(ctx context.Context) (string, error) { + // Use gh CLI to get repository info + cmd := exec.CommandContext(ctx, "gh", "repo", "view", "--json", "nameWithOwner", "-q", ".nameWithOwner") + output, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("gh repo view: %w", err) + } + return string(bytes.TrimSpace(output)), nil +} + +// postSingleReviewComment posts a single review comment using GitHub API. +func postSingleReviewComment(ctx context.Context, repo string, comment PRReviewComment) error { + // Prepare the API request body + requestBody := map[string]interface{}{ + "body": comment.Body, + "commit_id": comment.CommitID, + "path": comment.FilePath, + "line": comment.LineNumber, + } + + requestJSON, err := json.Marshal(requestBody) + if err != nil { + return fmt.Errorf("marshal request: %w", err) + } + + // Use gh CLI to make the API request + // gh api repos/{owner}/{repo}/pulls/{pr_number}/comments -X POST --input - + cmd := exec.CommandContext( + ctx, + "gh", "api", + fmt.Sprintf("repos/%s/pulls/%s/comments", repo, comment.PRNumber), + "-X", "POST", + "--input", "-", + ) + + cmd.Stdin = bytes.NewReader(requestJSON) + + var stderr bytes.Buffer + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + // Check if GITHUB_TOKEN is set + if os.Getenv("GITHUB_TOKEN") == "" { + return fmt.Errorf("GITHUB_TOKEN environment variable not set") + } + return fmt.Errorf("gh api failed: %w (stderr: %s)", err, stderr.String()) + } + + return nil +} diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go new file mode 100644 index 00000000..5707d28b --- /dev/null +++ b/cmd/commentprcasdiff/main.go @@ -0,0 +1,150 @@ +// Copyright 2021-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "fmt" + "os" +) + +func main() { + if err := run(context.Background()); err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } +} + +func run(ctx context.Context) error { + // Read environment variables + prNumber := os.Getenv("PR_NUMBER") + baseRef := os.Getenv("BASE_REF") + headRef := os.Getenv("HEAD_REF") + + if prNumber == "" { + return fmt.Errorf("PR_NUMBER environment variable is required") + } + if baseRef == "" { + return fmt.Errorf("BASE_REF environment variable is required") + } + if headRef == "" { + return fmt.Errorf("HEAD_REF environment variable is required") + } + + fmt.Fprintf(os.Stdout, "Processing PR #%s (base: %s, head: %s)\n", prNumber, baseRef, headRef) + + // Find changed module state directories + modulePaths, err := ChangedModuleStatesFromPR(ctx, prNumber, baseRef, headRef) + if err != nil { + return fmt.Errorf("find changed modules: %w", err) + } + + if len(modulePaths) == 0 { + fmt.Fprintf(os.Stdout, "No module state.json files changed in this PR\n") + return nil + } + + fmt.Fprintf(os.Stdout, "Found %d changed module(s)\n", len(modulePaths)) + + // Collect all transitions from all modules + var allTransitions []StateTransition + for _, modulePath := range modulePaths { + stateFilePath := modulePath + "/state.json" + fmt.Fprintf(os.Stdout, "Analyzing %s...\n", stateFilePath) + + transitions, err := GetStateFileTransitions(ctx, stateFilePath, baseRef, headRef) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to analyze %s: %v\n", stateFilePath, err) + continue + } + + if len(transitions) > 0 { + fmt.Fprintf(os.Stdout, " Found %d digest transition(s)\n", len(transitions)) + allTransitions = append(allTransitions, transitions...) + } else { + fmt.Fprintf(os.Stdout, " No digest changes\n") + } + } + + if len(allTransitions) == 0 { + fmt.Fprintf(os.Stdout, "No digest transitions found\n") + return nil + } + + fmt.Fprintf(os.Stdout, "\nRunning casdiff for %d transition(s)...\n", len(allTransitions)) + + // Run casdiff for all transitions in parallel + results := RunCASDiffParallel(ctx, allTransitions) + + // Separate successful results from errors + var ( + successfulResults []CASDiffResult + failedResults []CASDiffResult + ) + + for _, result := range results { + if result.Error != nil { + failedResults = append(failedResults, result) + fmt.Fprintf( + os.Stderr, "Warning: casdiff failed for %s %s->%s: %v\n", + result.Transition.ModulePath, + result.Transition.FromRef, + result.Transition.ToRef, + result.Error, + ) + } else { + successfulResults = append(successfulResults, result) + } + } + + // Post comments for successful results + if len(successfulResults) > 0 { + fmt.Fprintf(os.Stdout, "\nPosting %d comment(s) to PR...\n", len(successfulResults)) + + comments := make([]PRReviewComment, len(successfulResults)) + for i, result := range successfulResults { + comments[i] = PRReviewComment{ + PRNumber: prNumber, + FilePath: result.Transition.FilePath, + LineNumber: result.Transition.LineNumber, + Body: result.Output, + CommitID: headRef, + } + } + + errors := PostReviewComments(ctx, comments...) + for _, err := range errors { + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to post comment: %v\n", err) + } + } + + fmt.Fprintf(os.Stdout, "Successfully posted %d comment(s)\n", len(successfulResults)-len(errors)) + } + + // Log summary of failures + if len(failedResults) > 0 { + fmt.Fprintf(os.Stderr, "\nSummary: %d casdiff command(s) failed:\n", len(failedResults)) + for _, result := range failedResults { + fmt.Fprintf(os.Stderr, " - %s: %s -> %s\n", + result.Transition.ModulePath, + result.Transition.FromRef, + result.Transition.ToRef) + } + } + + fmt.Fprintf(os.Stdout, "\nDone!\n") + return nil +} diff --git a/cmd/commentprcasdiff/module_finder.go b/cmd/commentprcasdiff/module_finder.go new file mode 100644 index 00000000..3d126a91 --- /dev/null +++ b/cmd/commentprcasdiff/module_finder.go @@ -0,0 +1,62 @@ +// Copyright 2021-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "fmt" + "os/exec" + "path/filepath" + "strings" +) + +// ChangedModuleStatesFromPR returns module state directories that changed in the PR. +// Returns paths like "modules/sync/bufbuild/protovalidate" (without /state.json suffix). +func ChangedModuleStatesFromPR( + ctx context.Context, + prNumber string, + baseRef string, + headRef string, +) ([]string, error) { + // Get list of changed files in the PR + cmd := exec.CommandContext(ctx, "git", "diff", "--name-only", baseRef, headRef) + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("git diff: %w", err) + } + + // Filter for state.json files in modules/sync/ + var modulePaths []string + seen := make(map[string]bool) + + for _, line := range strings.Split(string(output), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + + // Look for state.json files in modules/sync/ + if strings.HasPrefix(line, "modules/sync/") && strings.HasSuffix(line, "/state.json") { + // Extract module path (remove /state.json suffix) + modulePath := filepath.Dir(line) + if !seen[modulePath] { + seen[modulePath] = true + modulePaths = append(modulePaths, modulePath) + } + } + } + + return modulePaths, nil +} diff --git a/cmd/commentprcasdiff/state_analyzer.go b/cmd/commentprcasdiff/state_analyzer.go new file mode 100644 index 00000000..22953986 --- /dev/null +++ b/cmd/commentprcasdiff/state_analyzer.go @@ -0,0 +1,222 @@ +// Copyright 2021-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "os/exec" + "path/filepath" + "strings" +) + +// StateTransition represents a digest change in a module's state.json file. +type StateTransition struct { + ModulePath string // e.g., "modules/sync/bufbuild/protovalidate" + FilePath string // e.g., "modules/sync/bufbuild/protovalidate/state.json" + FromRef string // Last reference with old digest (e.g., "v1.1.0") + ToRef string // First reference with new digest (e.g., "v1.2.0") + FromDigest string // Old digest + ToDigest string // New digest + LineNumber int // Line in diff where new digest first appears +} + +type moduleState struct { + References []reference `json:"references"` +} + +type reference struct { + Name string `json:"name"` + Digest string `json:"digest"` +} + +// GetStateFileTransitions reads state.json from base and head branches, +// compares the JSON arrays to find appended references, and detects digest transitions. +func GetStateFileTransitions( + ctx context.Context, + filePath string, + baseRef string, + headRef string, +) ([]StateTransition, error) { + // Read state.json from both branches + baseContent, err := readFileAtRef(ctx, filePath, baseRef) + if err != nil { + return nil, fmt.Errorf("read base state: %w", err) + } + + headContent, err := readFileAtRef(ctx, filePath, headRef) + if err != nil { + return nil, fmt.Errorf("read head state: %w", err) + } + + // Parse JSON + var baseState, headState moduleState + if err := json.Unmarshal(baseContent, &baseState); err != nil { + return nil, fmt.Errorf("parse base state JSON: %w", err) + } + if err := json.Unmarshal(headContent, &headState); err != nil { + return nil, fmt.Errorf("parse head state JSON: %w", err) + } + + // Identify appended references + baseCount := len(baseState.References) + headCount := len(headState.References) + + if headCount <= baseCount { + // No new references added + return nil, nil + } + + appendedRefs := headState.References[baseCount:] + + // Get the baseline digest (last reference in base state) + var ( + currentDigest string + currentRef string + ) + if baseCount > 0 { + lastBaseRef := baseState.References[baseCount-1] + currentDigest = lastBaseRef.Digest + currentRef = lastBaseRef.Name + } else { + // If base state is empty, use first appended ref as baseline + if len(appendedRefs) > 0 { + currentDigest = appendedRefs[0].Digest + currentRef = appendedRefs[0].Name + appendedRefs = appendedRefs[1:] // Skip first as it's now the baseline + } + } + + // Get line number mapping for the appended references + lineNumbers, err := getLineNumbersForAppendedRefs(ctx, filePath, baseRef, headRef, baseCount, headCount) + if err != nil { + return nil, fmt.Errorf("get line numbers: %w", err) + } + + // Detect digest transitions + var transitions []StateTransition + modulePath := filepath.Dir(filePath) + + for i, ref := range appendedRefs { + if ref.Digest != currentDigest { + // Digest changed! Record transition + lineNumber := 0 + if i < len(lineNumbers) { + lineNumber = lineNumbers[i] + } + + transition := StateTransition{ + ModulePath: modulePath, + FilePath: filePath, + FromRef: currentRef, + ToRef: ref.Name, + FromDigest: currentDigest, + ToDigest: ref.Digest, + LineNumber: lineNumber, + } + transitions = append(transitions, transition) + + // Update current for next iteration + currentDigest = ref.Digest + } + currentRef = ref.Name + } + + return transitions, nil +} + +// readFileAtRef reads a file's content at a specific git ref using git show. +func readFileAtRef(ctx context.Context, filePath string, ref string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "git", "show", fmt.Sprintf("%s:%s", ref, filePath)) + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("git show %s:%s: %w", ref, filePath, err) + } + return output, nil +} + +// getLineNumbersForAppendedRefs calculates the line numbers in the diff where each +// appended reference's digest appears. +func getLineNumbersForAppendedRefs( + ctx context.Context, + filePath string, + baseRef string, + headRef string, + baseCount int, + headCount int, +) ([]int, error) { + // Get the unified diff + cmd := exec.CommandContext(ctx, "git", "diff", "-U0", baseRef, headRef, "--", filePath) + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("git diff: %w", err) + } + + return parseLineNumbersFromDiff(string(output), headCount-baseCount) +} + +// parseLineNumbersFromDiff parses a git diff output and extracts line numbers +// where "digest" fields appear in added lines. +func parseLineNumbersFromDiff(diffOutput string, expectedCount int) ([]int, error) { + lineNumbers := make([]int, expectedCount) + scanner := bufio.NewScanner(strings.NewReader(diffOutput)) + + var ( + currentLine int + refIndex = 0 + inAddedSection = false + ) + + for scanner.Scan() { + line := scanner.Text() + + // Parse hunk headers like: @@ -275,0 +276,12 @@ + if strings.HasPrefix(line, "@@") { + parts := strings.Split(line, " ") + if len(parts) >= 3 { + // Extract new file line number from +276,12 + newRange := strings.TrimPrefix(parts[2], "+") + newRange = strings.Split(newRange, ",")[0] + fmt.Sscanf(newRange, "%d", ¤tLine) + inAddedSection = true + continue + } + } + + if !inAddedSection { + continue + } + + // Look for added lines (starting with +) + if strings.HasPrefix(line, "+") && !strings.HasPrefix(line, "+++") { + // Check if this line contains "digest" + if strings.Contains(line, `"digest"`) { + if refIndex < len(lineNumbers) { + lineNumbers[refIndex] = currentLine + refIndex++ + } + } + currentLine++ + } + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("scan diff: %w", err) + } + + return lineNumbers, nil +} diff --git a/cmd/commentprcasdiff/state_analyzer_test.go b/cmd/commentprcasdiff/state_analyzer_test.go new file mode 100644 index 00000000..738d3a28 --- /dev/null +++ b/cmd/commentprcasdiff/state_analyzer_test.go @@ -0,0 +1,119 @@ +// Copyright 2021-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "reflect" + "testing" +) + +func TestParseLineNumbersFromDiff(t *testing.T) { + tests := []struct { + name string + diffOutput string + expectedCount int + want []int + wantErr bool + }{ + { + name: "single_digest_change", + diffOutput: `@@ -275,0 +276,6 @@ ++ }, ++ { ++ "name": "v1.1.0", ++ "digest": "35b3d88f6b0fbf159d9eedfc2fbfa976490e6bca1d98914c1c71f29bfe2da6261fca56c057975b3e5c022b3234a0f2eea8e2d1b599a937c6c5d63d21201a9bc3" ++ } ++ ]`, + expectedCount: 1, + want: []int{279}, + }, + { + name: "multiple_digest_changes", + diffOutput: `@@ -100,0 +101,12 @@ ++ }, ++ { ++ "name": "v1.1.0", ++ "digest": "aaa" ++ }, ++ { ++ "name": "v1.2.0", ++ "digest": "bbb" ++ }, ++ { ++ "name": "v1.3.0", ++ "digest": "ccc"`, + expectedCount: 3, + want: []int{104, 108, 112}, + }, + { + name: "multiple_hunks", + diffOutput: `@@ -50,0 +51,4 @@ ++ { ++ "name": "v2.0.0", ++ "digest": "xxx" ++ } +@@ -100,0 +105,4 @@ ++ { ++ "name": "v3.0.0", ++ "digest": "yyy" ++ }`, + expectedCount: 2, + want: []int{53, 107}, + }, + { + name: "no_digest_lines", + diffOutput: `@@ -10,0 +11,4 @@ ++ { ++ "name": "v1.0.0", ++ "other": "field" ++ }`, + expectedCount: 1, + want: []int{0}, + }, + { + name: "empty_diff", + diffOutput: "", + expectedCount: 1, + want: []int{0}, + }, + { + name: "digest_with_spaces_and_formatting", + diffOutput: `@@ -200,0 +201,8 @@ ++ { ++ "name": "v0.1.0", ++ "digest": "abc123" ++ }, ++ { ++ "name": "v0.2.0", ++ "digest": "def456" ++ }`, + expectedCount: 2, + want: []int{203, 207}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseLineNumbersFromDiff(tt.diffOutput, tt.expectedCount) + if (err != nil) != tt.wantErr { + t.Errorf("parseLineNumbersFromDiff() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("parseLineNumbersFromDiff() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/cmd/release/main.go b/cmd/release/main.go index f66dc25f..051bcc9e 100644 --- a/cmd/release/main.go +++ b/cmd/release/main.go @@ -306,9 +306,9 @@ func createRelease( githubutil.GithubOwnerBufbuild, githubutil.GithubRepoModules, &github.RepositoryRelease{ - TagName: new(releaseName), - Name: new(releaseName), - Body: new(releaseBody), + TagName: &releaseName, + Name: &releaseName, + Body: &releaseBody, Draft: new(true), // Start release as a draft until all assets are uploaded }) if err != nil { From 870975d75ec0c6be8125a0866334cd486b118aa0 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:08:44 -0500 Subject: [PATCH 02/37] lint --- cmd/commentprcasdiff/casdiff_runner.go | 2 +- cmd/commentprcasdiff/comment_poster.go | 7 ++++--- cmd/commentprcasdiff/main.go | 7 ++++--- cmd/commentprcasdiff/module_finder.go | 6 +++--- cmd/commentprcasdiff/state_analyzer.go | 18 +++++++++--------- cmd/commentprcasdiff/state_analyzer_test.go | 2 ++ 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/cmd/commentprcasdiff/casdiff_runner.go b/cmd/commentprcasdiff/casdiff_runner.go index d0d1dcec..77b37af7 100644 --- a/cmd/commentprcasdiff/casdiff_runner.go +++ b/cmd/commentprcasdiff/casdiff_runner.go @@ -36,7 +36,7 @@ func RunCASDiff(ctx context.Context, transition StateTransition) CASDiffResult { } // Run casdiff in the module directory - cmd := exec.CommandContext( + cmd := exec.CommandContext( //nolint:gosec ctx, "go", "run", "../../cmd/casdiff", transition.FromRef, diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index fb31d701..256b1745 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "os" "os/exec" @@ -72,7 +73,7 @@ func getRepositoryInfo(ctx context.Context) (string, error) { // postSingleReviewComment posts a single review comment using GitHub API. func postSingleReviewComment(ctx context.Context, repo string, comment PRReviewComment) error { // Prepare the API request body - requestBody := map[string]interface{}{ + requestBody := map[string]any{ "body": comment.Body, "commit_id": comment.CommitID, "path": comment.FilePath, @@ -86,7 +87,7 @@ func postSingleReviewComment(ctx context.Context, repo string, comment PRReviewC // Use gh CLI to make the API request // gh api repos/{owner}/{repo}/pulls/{pr_number}/comments -X POST --input - - cmd := exec.CommandContext( + cmd := exec.CommandContext( //nolint:gosec ctx, "gh", "api", fmt.Sprintf("repos/%s/pulls/%s/comments", repo, comment.PRNumber), @@ -102,7 +103,7 @@ func postSingleReviewComment(ctx context.Context, repo string, comment PRReviewC if err := cmd.Run(); err != nil { // Check if GITHUB_TOKEN is set if os.Getenv("GITHUB_TOKEN") == "" { - return fmt.Errorf("GITHUB_TOKEN environment variable not set") + return errors.New("GITHUB_TOKEN environment variable not set") } return fmt.Errorf("gh api failed: %w (stderr: %s)", err, stderr.String()) } diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index 5707d28b..f58cea22 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -16,6 +16,7 @@ package main import ( "context" + "errors" "fmt" "os" ) @@ -34,13 +35,13 @@ func run(ctx context.Context) error { headRef := os.Getenv("HEAD_REF") if prNumber == "" { - return fmt.Errorf("PR_NUMBER environment variable is required") + return errors.New("PR_NUMBER environment variable is required") } if baseRef == "" { - return fmt.Errorf("BASE_REF environment variable is required") + return errors.New("BASE_REF environment variable is required") } if headRef == "" { - return fmt.Errorf("HEAD_REF environment variable is required") + return errors.New("HEAD_REF environment variable is required") } fmt.Fprintf(os.Stdout, "Processing PR #%s (base: %s, head: %s)\n", prNumber, baseRef, headRef) diff --git a/cmd/commentprcasdiff/module_finder.go b/cmd/commentprcasdiff/module_finder.go index 3d126a91..6355a31f 100644 --- a/cmd/commentprcasdiff/module_finder.go +++ b/cmd/commentprcasdiff/module_finder.go @@ -26,12 +26,12 @@ import ( // Returns paths like "modules/sync/bufbuild/protovalidate" (without /state.json suffix). func ChangedModuleStatesFromPR( ctx context.Context, - prNumber string, + _ string, baseRef string, headRef string, ) ([]string, error) { // Get list of changed files in the PR - cmd := exec.CommandContext(ctx, "git", "diff", "--name-only", baseRef, headRef) + cmd := exec.CommandContext(ctx, "git", "diff", "--name-only", baseRef, headRef) //nolint:gosec output, err := cmd.Output() if err != nil { return nil, fmt.Errorf("git diff: %w", err) @@ -41,7 +41,7 @@ func ChangedModuleStatesFromPR( var modulePaths []string seen := make(map[string]bool) - for _, line := range strings.Split(string(output), "\n") { + for line := range strings.SplitSeq(string(output), "\n") { line = strings.TrimSpace(line) if line == "" { continue diff --git a/cmd/commentprcasdiff/state_analyzer.go b/cmd/commentprcasdiff/state_analyzer.go index 22953986..45dcf14c 100644 --- a/cmd/commentprcasdiff/state_analyzer.go +++ b/cmd/commentprcasdiff/state_analyzer.go @@ -92,13 +92,11 @@ func GetStateFileTransitions( lastBaseRef := baseState.References[baseCount-1] currentDigest = lastBaseRef.Digest currentRef = lastBaseRef.Name - } else { + } else if len(appendedRefs) > 0 { // If base state is empty, use first appended ref as baseline - if len(appendedRefs) > 0 { - currentDigest = appendedRefs[0].Digest - currentRef = appendedRefs[0].Name - appendedRefs = appendedRefs[1:] // Skip first as it's now the baseline - } + currentDigest = appendedRefs[0].Digest + currentRef = appendedRefs[0].Name + appendedRefs = appendedRefs[1:] // Skip first as it's now the baseline } // Get line number mapping for the appended references @@ -141,7 +139,7 @@ func GetStateFileTransitions( // readFileAtRef reads a file's content at a specific git ref using git show. func readFileAtRef(ctx context.Context, filePath string, ref string) ([]byte, error) { - cmd := exec.CommandContext(ctx, "git", "show", fmt.Sprintf("%s:%s", ref, filePath)) + cmd := exec.CommandContext(ctx, "git", "show", fmt.Sprintf("%s:%s", ref, filePath)) //nolint:gosec output, err := cmd.Output() if err != nil { return nil, fmt.Errorf("git show %s:%s: %w", ref, filePath, err) @@ -160,7 +158,7 @@ func getLineNumbersForAppendedRefs( headCount int, ) ([]int, error) { // Get the unified diff - cmd := exec.CommandContext(ctx, "git", "diff", "-U0", baseRef, headRef, "--", filePath) + cmd := exec.CommandContext(ctx, "git", "diff", "-U0", baseRef, headRef, "--", filePath) //nolint:gosec output, err := cmd.Output() if err != nil { return nil, fmt.Errorf("git diff: %w", err) @@ -191,7 +189,9 @@ func parseLineNumbersFromDiff(diffOutput string, expectedCount int) ([]int, erro // Extract new file line number from +276,12 newRange := strings.TrimPrefix(parts[2], "+") newRange = strings.Split(newRange, ",")[0] - fmt.Sscanf(newRange, "%d", ¤tLine) + if _, err := fmt.Sscanf(newRange, "%d", ¤tLine); err != nil { + return nil, fmt.Errorf("parse hunk header line number %q: %w", newRange, err) + } inAddedSection = true continue } diff --git a/cmd/commentprcasdiff/state_analyzer_test.go b/cmd/commentprcasdiff/state_analyzer_test.go index 738d3a28..c6719806 100644 --- a/cmd/commentprcasdiff/state_analyzer_test.go +++ b/cmd/commentprcasdiff/state_analyzer_test.go @@ -20,6 +20,7 @@ import ( ) func TestParseLineNumbersFromDiff(t *testing.T) { + t.Parallel() tests := []struct { name string diffOutput string @@ -106,6 +107,7 @@ func TestParseLineNumbersFromDiff(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() got, err := parseLineNumbersFromDiff(tt.diffOutput, tt.expectedCount) if (err != nil) != tt.wantErr { t.Errorf("parseLineNumbersFromDiff() error = %v, wantErr %v", err, tt.wantErr) From 03b780b2b7caaa58f75c8bed06670aac467e1851 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:16:50 -0500 Subject: [PATCH 03/37] Allow on any PR. --- .github/workflows/auto-casdiff-comment.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/auto-casdiff-comment.yaml b/.github/workflows/auto-casdiff-comment.yaml index 7bb887f7..757ad886 100644 --- a/.github/workflows/auto-casdiff-comment.yaml +++ b/.github/workflows/auto-casdiff-comment.yaml @@ -3,8 +3,9 @@ name: Auto-comment PR with casdiff on: pull_request: types: [opened, synchronize] - branches: - - fetch-modules + # FIXME: re-enable once tested + # branches: + # - main permissions: contents: read From 6a16eae7a861b394ff70323ced502349d2e156c8 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:18:52 -0500 Subject: [PATCH 04/37] Remove some state changes --- .../protocolbuffers/wellknowntypes/state.json | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/modules/sync/protocolbuffers/wellknowntypes/state.json b/modules/sync/protocolbuffers/wellknowntypes/state.json index ddbce932..a6ba7303 100644 --- a/modules/sync/protocolbuffers/wellknowntypes/state.json +++ b/modules/sync/protocolbuffers/wellknowntypes/state.json @@ -463,30 +463,6 @@ { "name": "v33.0", "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" - }, - { - "name": "v33.1", - "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" - }, - { - "name": "v33.2", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.3", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.4", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.5", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v34.0", - "digest": "946278992baee216f7742401780df3727195ffc068a30e21769c7e808d16ee25e9e48e51f938b6ddbe405abe46c45091a56ad90aac1cad0d809d16cbad889ff1" } ] -} \ No newline at end of file +} From 918c3f0fc872a6e6114c4383ec063c5564c45b9e Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:20:12 -0500 Subject: [PATCH 05/37] Revert "Remove some state changes" This reverts commit 6a16eae7a861b394ff70323ced502349d2e156c8. --- .../protocolbuffers/wellknowntypes/state.json | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/modules/sync/protocolbuffers/wellknowntypes/state.json b/modules/sync/protocolbuffers/wellknowntypes/state.json index a6ba7303..ddbce932 100644 --- a/modules/sync/protocolbuffers/wellknowntypes/state.json +++ b/modules/sync/protocolbuffers/wellknowntypes/state.json @@ -463,6 +463,30 @@ { "name": "v33.0", "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" + }, + { + "name": "v33.1", + "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" + }, + { + "name": "v33.2", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.3", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.4", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.5", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v34.0", + "digest": "946278992baee216f7742401780df3727195ffc068a30e21769c7e808d16ee25e9e48e51f938b6ddbe405abe46c45091a56ad90aac1cad0d809d16cbad889ff1" } ] -} +} \ No newline at end of file From 931a56896278053b43a4a268b872d7145f4f9e4e Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:21:43 -0500 Subject: [PATCH 06/37] Newer go --- .github/workflows/auto-casdiff-comment.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/auto-casdiff-comment.yaml b/.github/workflows/auto-casdiff-comment.yaml index 757ad886..017948c2 100644 --- a/.github/workflows/auto-casdiff-comment.yaml +++ b/.github/workflows/auto-casdiff-comment.yaml @@ -24,7 +24,7 @@ jobs: - name: Install Go uses: actions/setup-go@v6 with: - go-version: 1.24.x + go-version: 1.26.x check-latest: true cache: true From 70353b3b12d21f4e093a7c1d05433d11ae084d46 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:35:10 -0500 Subject: [PATCH 07/37] nits --- cmd/commentprcasdiff/casdiff_runner.go | 16 +++++++++++++--- cmd/commentprcasdiff/main.go | 21 +++++++++++++++------ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/cmd/commentprcasdiff/casdiff_runner.go b/cmd/commentprcasdiff/casdiff_runner.go index 77b37af7..3573df7f 100644 --- a/cmd/commentprcasdiff/casdiff_runner.go +++ b/cmd/commentprcasdiff/casdiff_runner.go @@ -18,7 +18,9 @@ import ( "bytes" "context" "fmt" + "os" "os/exec" + "path/filepath" "sync" ) @@ -35,15 +37,23 @@ func RunCASDiff(ctx context.Context, transition StateTransition) CASDiffResult { Transition: transition, } - // Run casdiff in the module directory + repoRoot, err := os.Getwd() + if err != nil { + result.Error = fmt.Errorf("get working directory: %w", err) + return result + } + + // Run casdiff in the module directory. casdiff reads state.json from "." so it must + // run from the module directory. We use an absolute path to the package to avoid + // path resolution issues when cmd.Dir is set. cmd := exec.CommandContext( //nolint:gosec ctx, - "go", "run", "../../cmd/casdiff", + "go", "run", filepath.Join(repoRoot, "cmd", "casdiff"), transition.FromRef, transition.ToRef, "--format=markdown", ) - cmd.Dir = transition.ModulePath + cmd.Dir = filepath.Join(repoRoot, transition.ModulePath) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index f58cea22..4eaccfe3 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -111,6 +111,7 @@ func run(ctx context.Context) error { } // Post comments for successful results + var postErrs []error if len(successfulResults) > 0 { fmt.Fprintf(os.Stdout, "\nPosting %d comment(s) to PR...\n", len(successfulResults)) @@ -125,17 +126,19 @@ func run(ctx context.Context) error { } } - errors := PostReviewComments(ctx, comments...) - for _, err := range errors { + postErrs = PostReviewComments(ctx, comments...) + posted := 0 + for _, err := range postErrs { if err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to post comment: %v\n", err) + fmt.Fprintf(os.Stderr, "Error: failed to post comment: %v\n", err) + } else { + posted++ } } - - fmt.Fprintf(os.Stdout, "Successfully posted %d comment(s)\n", len(successfulResults)-len(errors)) + fmt.Fprintf(os.Stdout, "Successfully posted %d comment(s)\n", posted) } - // Log summary of failures + // Report failures and exit non-zero if anything went wrong. if len(failedResults) > 0 { fmt.Fprintf(os.Stderr, "\nSummary: %d casdiff command(s) failed:\n", len(failedResults)) for _, result := range failedResults { @@ -144,6 +147,12 @@ func run(ctx context.Context) error { result.Transition.FromRef, result.Transition.ToRef) } + return fmt.Errorf("%d casdiff command(s) failed", len(failedResults)) + } + for _, err := range postErrs { + if err != nil { + return fmt.Errorf("one or more comments failed to post") + } } fmt.Fprintf(os.Stdout, "\nDone!\n") From e2e0336ad3381711b5424f04d19e1f77c1224fea Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:38:52 -0500 Subject: [PATCH 08/37] add dry-run --- cmd/commentprcasdiff/main.go | 60 +++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index 4eaccfe3..bf3420ad 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -17,6 +17,7 @@ package main import ( "context" "errors" + "flag" "fmt" "os" ) @@ -29,12 +30,16 @@ func main() { } func run(ctx context.Context) error { + var dryRun bool + flag.BoolVar(&dryRun, "dry-run", false, "print comments to stdout instead of posting to GitHub") + flag.Parse() + // Read environment variables prNumber := os.Getenv("PR_NUMBER") baseRef := os.Getenv("BASE_REF") headRef := os.Getenv("HEAD_REF") - if prNumber == "" { + if prNumber == "" && !dryRun { return errors.New("PR_NUMBER environment variable is required") } if baseRef == "" { @@ -110,32 +115,45 @@ func run(ctx context.Context) error { } } - // Post comments for successful results + // Post or print comments for successful results var postErrs []error if len(successfulResults) > 0 { - fmt.Fprintf(os.Stdout, "\nPosting %d comment(s) to PR...\n", len(successfulResults)) - - comments := make([]PRReviewComment, len(successfulResults)) - for i, result := range successfulResults { - comments[i] = PRReviewComment{ - PRNumber: prNumber, - FilePath: result.Transition.FilePath, - LineNumber: result.Transition.LineNumber, - Body: result.Output, - CommitID: headRef, + if dryRun { + fmt.Fprintf(os.Stdout, "\n[dry-run] %d comment(s) would be posted:\n", len(successfulResults)) + for _, result := range successfulResults { + fmt.Fprintf(os.Stdout, "\n--- %s (line %d: %s -> %s) ---\n%s\n", + result.Transition.FilePath, + result.Transition.LineNumber, + result.Transition.FromRef, + result.Transition.ToRef, + result.Output, + ) + } + } else { + fmt.Fprintf(os.Stdout, "\nPosting %d comment(s) to PR...\n", len(successfulResults)) + + comments := make([]PRReviewComment, len(successfulResults)) + for i, result := range successfulResults { + comments[i] = PRReviewComment{ + PRNumber: prNumber, + FilePath: result.Transition.FilePath, + LineNumber: result.Transition.LineNumber, + Body: result.Output, + CommitID: headRef, + } } - } - postErrs = PostReviewComments(ctx, comments...) - posted := 0 - for _, err := range postErrs { - if err != nil { - fmt.Fprintf(os.Stderr, "Error: failed to post comment: %v\n", err) - } else { - posted++ + postErrs = PostReviewComments(ctx, comments...) + posted := 0 + for _, err := range postErrs { + if err != nil { + fmt.Fprintf(os.Stderr, "Error: failed to post comment: %v\n", err) + } else { + posted++ + } } + fmt.Fprintf(os.Stdout, "Successfully posted %d comment(s)\n", posted) } - fmt.Fprintf(os.Stdout, "Successfully posted %d comment(s)\n", posted) } // Report failures and exit non-zero if anything went wrong. From 1f27022ce844e2ed38a7b7b8fcc2bc37fc951b67 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:42:56 -0500 Subject: [PATCH 09/37] lint --- cmd/commentprcasdiff/state_analyzer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/commentprcasdiff/state_analyzer.go b/cmd/commentprcasdiff/state_analyzer.go index 45dcf14c..16dcfb94 100644 --- a/cmd/commentprcasdiff/state_analyzer.go +++ b/cmd/commentprcasdiff/state_analyzer.go @@ -190,8 +190,8 @@ func parseLineNumbersFromDiff(diffOutput string, expectedCount int) ([]int, erro newRange := strings.TrimPrefix(parts[2], "+") newRange = strings.Split(newRange, ",")[0] if _, err := fmt.Sscanf(newRange, "%d", ¤tLine); err != nil { - return nil, fmt.Errorf("parse hunk header line number %q: %w", newRange, err) - } + return nil, fmt.Errorf("parse hunk header line number %q: %w", newRange, err) + } inAddedSection = true continue } From be9ed5dcf10e1a021cf3cf6481f79a6320f5bcd2 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 16:11:42 -0500 Subject: [PATCH 10/37] nits --- cmd/commentprcasdiff/main.go | 15 +++++++++++---- cmd/commentprcasdiff/module_finder.go | 24 ++++++++++-------------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index bf3420ad..2a4981ae 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -15,6 +15,7 @@ package main import ( + "cmp" "context" "errors" "flag" @@ -40,7 +41,7 @@ func run(ctx context.Context) error { headRef := os.Getenv("HEAD_REF") if prNumber == "" && !dryRun { - return errors.New("PR_NUMBER environment variable is required") + return errors.New("PR_NUMBER environment variable is required when not a dry-run") } if baseRef == "" { return errors.New("BASE_REF environment variable is required") @@ -49,16 +50,22 @@ func run(ctx context.Context) error { return errors.New("HEAD_REF environment variable is required") } - fmt.Fprintf(os.Stdout, "Processing PR #%s (base: %s, head: %s)\n", prNumber, baseRef, headRef) + fmt.Fprintf( + os.Stdout, + "Processing PR #%s (base: %s, head: %s)\n", + cmp.Or(prNumber, "dry-run"), + baseRef, + headRef, + ) // Find changed module state directories - modulePaths, err := ChangedModuleStatesFromPR(ctx, prNumber, baseRef, headRef) + modulePaths, err := changedModuleStates(ctx, baseRef, headRef) if err != nil { return fmt.Errorf("find changed modules: %w", err) } if len(modulePaths) == 0 { - fmt.Fprintf(os.Stdout, "No module state.json files changed in this PR\n") + fmt.Fprintf(os.Stdout, "No module state.json files changed in between base..head refs\n") return nil } diff --git a/cmd/commentprcasdiff/module_finder.go b/cmd/commentprcasdiff/module_finder.go index 6355a31f..345b8fc1 100644 --- a/cmd/commentprcasdiff/module_finder.go +++ b/cmd/commentprcasdiff/module_finder.go @@ -20,13 +20,15 @@ import ( "os/exec" "path/filepath" "strings" + + "buf.build/go/standard/xslices" ) -// ChangedModuleStatesFromPR returns module state directories that changed in the PR. -// Returns paths like "modules/sync/bufbuild/protovalidate" (without /state.json suffix). -func ChangedModuleStatesFromPR( +// changedModuleStates returns module directories that had *any change* on its state.json files +// between the passed base and head refs. Returns paths like "modules/sync/bufbuild/protovalidate" +// (without /state.json suffix). +func changedModuleStates( ctx context.Context, - _ string, baseRef string, headRef string, ) ([]string, error) { @@ -38,25 +40,19 @@ func ChangedModuleStatesFromPR( } // Filter for state.json files in modules/sync/ - var modulePaths []string - seen := make(map[string]bool) - + modulePaths := make(map[string]struct{}) for line := range strings.SplitSeq(string(output), "\n") { line = strings.TrimSpace(line) if line == "" { continue } - - // Look for state.json files in modules/sync/ + // Look for "modules/sync///state.json" files if strings.HasPrefix(line, "modules/sync/") && strings.HasSuffix(line, "/state.json") { // Extract module path (remove /state.json suffix) modulePath := filepath.Dir(line) - if !seen[modulePath] { - seen[modulePath] = true - modulePaths = append(modulePaths, modulePath) - } + modulePaths[modulePath] = struct{}{} } } - return modulePaths, nil + return xslices.MapKeysToSortedSlice(modulePaths), nil } From 832b817b97372748e3682516ed546a6b3e1fdfbe Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 15:18:52 -0500 Subject: [PATCH 11/37] Remove some state changes --- .../protocolbuffers/wellknowntypes/state.json | 26 +------------------ modules/sync/state.json | 4 +-- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/modules/sync/protocolbuffers/wellknowntypes/state.json b/modules/sync/protocolbuffers/wellknowntypes/state.json index ddbce932..a6ba7303 100644 --- a/modules/sync/protocolbuffers/wellknowntypes/state.json +++ b/modules/sync/protocolbuffers/wellknowntypes/state.json @@ -463,30 +463,6 @@ { "name": "v33.0", "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" - }, - { - "name": "v33.1", - "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" - }, - { - "name": "v33.2", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.3", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.4", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.5", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v34.0", - "digest": "946278992baee216f7742401780df3727195ffc068a30e21769c7e808d16ee25e9e48e51f938b6ddbe405abe46c45091a56ad90aac1cad0d809d16cbad889ff1" } ] -} \ No newline at end of file +} diff --git a/modules/sync/state.json b/modules/sync/state.json index b3d5424d..b6c34896 100644 --- a/modules/sync/state.json +++ b/modules/sync/state.json @@ -78,7 +78,7 @@ }, { "module_name": "protocolbuffers/wellknowntypes", - "latest_reference": "v34.0" + "latest_reference": "v33.0" } ] -} \ No newline at end of file +} From 3ccf934bc99b701d18feb2b1577964a0bd04e520 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 16:12:45 -0500 Subject: [PATCH 12/37] Reapply "Remove some state changes" This reverts commit 918c3f0fc872a6e6114c4383ec063c5564c45b9e. --- .../protocolbuffers/wellknowntypes/state.json | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/modules/sync/protocolbuffers/wellknowntypes/state.json b/modules/sync/protocolbuffers/wellknowntypes/state.json index ddbce932..a6ba7303 100644 --- a/modules/sync/protocolbuffers/wellknowntypes/state.json +++ b/modules/sync/protocolbuffers/wellknowntypes/state.json @@ -463,30 +463,6 @@ { "name": "v33.0", "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" - }, - { - "name": "v33.1", - "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" - }, - { - "name": "v33.2", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.3", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.4", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.5", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v34.0", - "digest": "946278992baee216f7742401780df3727195ffc068a30e21769c7e808d16ee25e9e48e51f938b6ddbe405abe46c45091a56ad90aac1cad0d809d16cbad889ff1" } ] -} \ No newline at end of file +} From 610b51bc629160612bd59b548038a54d7e7efe73 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 16:12:56 -0500 Subject: [PATCH 13/37] Revert "Remove some state changes" This reverts commit 832b817b97372748e3682516ed546a6b3e1fdfbe. --- .../protocolbuffers/wellknowntypes/state.json | 26 ++++++++++++++++++- modules/sync/state.json | 4 +-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/modules/sync/protocolbuffers/wellknowntypes/state.json b/modules/sync/protocolbuffers/wellknowntypes/state.json index a6ba7303..ddbce932 100644 --- a/modules/sync/protocolbuffers/wellknowntypes/state.json +++ b/modules/sync/protocolbuffers/wellknowntypes/state.json @@ -463,6 +463,30 @@ { "name": "v33.0", "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" + }, + { + "name": "v33.1", + "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" + }, + { + "name": "v33.2", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.3", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.4", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.5", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v34.0", + "digest": "946278992baee216f7742401780df3727195ffc068a30e21769c7e808d16ee25e9e48e51f938b6ddbe405abe46c45091a56ad90aac1cad0d809d16cbad889ff1" } ] -} +} \ No newline at end of file diff --git a/modules/sync/state.json b/modules/sync/state.json index b6c34896..b3d5424d 100644 --- a/modules/sync/state.json +++ b/modules/sync/state.json @@ -78,7 +78,7 @@ }, { "module_name": "protocolbuffers/wellknowntypes", - "latest_reference": "v33.0" + "latest_reference": "v34.0" } ] -} +} \ No newline at end of file From e17638005cad593d1ac1892f07302a796a6e9cf2 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Tue, 17 Mar 2026 16:39:50 -0500 Subject: [PATCH 14/37] nit --- cmd/commentprcasdiff/casdiff_runner.go | 46 +++++++------- cmd/commentprcasdiff/comment_poster.go | 32 +++++----- cmd/commentprcasdiff/main.go | 85 +++++++++++++++----------- cmd/commentprcasdiff/module_finder.go | 26 +++----- cmd/commentprcasdiff/state_analyzer.go | 75 ++++++++++++----------- 5 files changed, 134 insertions(+), 130 deletions(-) diff --git a/cmd/commentprcasdiff/casdiff_runner.go b/cmd/commentprcasdiff/casdiff_runner.go index 3573df7f..c5ef0b2c 100644 --- a/cmd/commentprcasdiff/casdiff_runner.go +++ b/cmd/commentprcasdiff/casdiff_runner.go @@ -24,60 +24,60 @@ import ( "sync" ) -// CASDiffResult contains the result of running casdiff for a transition. -type CASDiffResult struct { - Transition StateTransition - Output string // Markdown output from casdiff - Error error +// casDiffResult contains the result of running casdiff for a transition. +type casDiffResult struct { + transition stateTransition + output string // Markdown output from casdiff + err error } -// RunCASDiff executes casdiff command in the module directory. -func RunCASDiff(ctx context.Context, transition StateTransition) CASDiffResult { - result := CASDiffResult{ - Transition: transition, +// runCASDiff executes casdiff command in the module directory. +func runCASDiff(ctx context.Context, transition stateTransition) casDiffResult { + result := casDiffResult{ + transition: transition, } repoRoot, err := os.Getwd() if err != nil { - result.Error = fmt.Errorf("get working directory: %w", err) + result.err = fmt.Errorf("get working directory: %w", err) return result } - // Run casdiff in the module directory. casdiff reads state.json from "." so it must - // run from the module directory. We use an absolute path to the package to avoid - // path resolution issues when cmd.Dir is set. + // Run casdiff in the module directory. casdiff reads state.json from "." so it must run from the + // module directory. We use an absolute path to the package to avoid path resolution issues when + // cmd.Dir is set. cmd := exec.CommandContext( //nolint:gosec ctx, "go", "run", filepath.Join(repoRoot, "cmd", "casdiff"), - transition.FromRef, - transition.ToRef, + transition.fromRef, + transition.toRef, "--format=markdown", ) - cmd.Dir = filepath.Join(repoRoot, transition.ModulePath) + cmd.Dir = filepath.Join(repoRoot, transition.modulePath) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - result.Error = fmt.Errorf("casdiff failed: %w (stderr: %s)", err, stderr.String()) + result.err = fmt.Errorf("casdiff failed: %w (stderr: %s)", err, stderr.String()) return result } - result.Output = stdout.String() + result.output = stdout.String() return result } -// RunCASDiffParallel runs multiple casdiff commands concurrently. -func RunCASDiffParallel(ctx context.Context, transitions []StateTransition) []CASDiffResult { - results := make([]CASDiffResult, len(transitions)) +// runCASDiffs runs multiple casdiff commands concurrently. +func runCASDiffs(ctx context.Context, transitions []stateTransition) []casDiffResult { + results := make([]casDiffResult, len(transitions)) var wg sync.WaitGroup for i, transition := range transitions { wg.Add(1) - go func(index int, trans StateTransition) { + go func(index int, trans stateTransition) { defer wg.Done() - results[index] = RunCASDiff(ctx, trans) + results[index] = runCASDiff(ctx, trans) }(i, transition) } diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index 256b1745..8b86e0f9 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -24,18 +24,18 @@ import ( "os/exec" ) -// PRReviewComment represents a comment to post on a specific line in a PR. -type PRReviewComment struct { - PRNumber string - FilePath string // File path in the PR (e.g., "modules/sync/bufbuild/protovalidate/state.json") - LineNumber int // Line number in the diff - Body string // Comment body (casdiff output) - CommitID string // Head commit SHA +// prReviewComment represents a comment to post on a specific line in a PR. +type prReviewComment struct { + prNumber string + filePath string // File path in the PR (e.g., "modules/sync/bufbuild/protovalidate/state.json") + lineNumber int // Line number in the diff + body string // Comment body (casdiff output) + commitID string // Head commit SHA } -// PostReviewComments posts review comments to specific lines in the PR diff. -// Uses GitHub API (via gh CLI) to create inline comments. -func PostReviewComments(ctx context.Context, comments ...PRReviewComment) []error { +// postReviewComments posts review comments to specific lines in the PR diff. Uses GitHub API (via +// gh CLI) to create inline comments. +func postReviewComments(ctx context.Context, comments ...prReviewComment) []error { var errors []error // Get repository information @@ -71,13 +71,13 @@ func getRepositoryInfo(ctx context.Context) (string, error) { } // postSingleReviewComment posts a single review comment using GitHub API. -func postSingleReviewComment(ctx context.Context, repo string, comment PRReviewComment) error { +func postSingleReviewComment(ctx context.Context, repo string, comment prReviewComment) error { // Prepare the API request body requestBody := map[string]any{ - "body": comment.Body, - "commit_id": comment.CommitID, - "path": comment.FilePath, - "line": comment.LineNumber, + "body": comment.body, + "commit_id": comment.commitID, + "path": comment.filePath, + "line": comment.lineNumber, } requestJSON, err := json.Marshal(requestBody) @@ -90,7 +90,7 @@ func postSingleReviewComment(ctx context.Context, repo string, comment PRReviewC cmd := exec.CommandContext( //nolint:gosec ctx, "gh", "api", - fmt.Sprintf("repos/%s/pulls/%s/comments", repo, comment.PRNumber), + fmt.Sprintf("repos/%s/pulls/%s/comments", repo, comment.prNumber), "-X", "POST", "--input", "-", ) diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index 2a4981ae..9fc7aef4 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -21,6 +21,9 @@ import ( "flag" "fmt" "os" + "strings" + + "buf.build/go/standard/xslices" ) func main() { @@ -59,27 +62,37 @@ func run(ctx context.Context) error { ) // Find changed module state directories - modulePaths, err := changedModuleStates(ctx, baseRef, headRef) + moduleStatePaths, err := changedModuleStateFiles(ctx, baseRef, headRef) if err != nil { return fmt.Errorf("find changed modules: %w", err) } - if len(modulePaths) == 0 { - fmt.Fprintf(os.Stdout, "No module state.json files changed in between base..head refs\n") + if len(moduleStatePaths) == 0 { + fmt.Fprintf( + os.Stdout, + "No module state.json files changed in between %s..%s refs\n", + baseRef, + headRef, + ) return nil } + moduleStatePathsSorted := xslices.MapKeysToSortedSlice(moduleStatePaths) - fmt.Fprintf(os.Stdout, "Found %d changed module(s)\n", len(modulePaths)) + fmt.Fprintf( + os.Stdout, + "Found %d changed module(s):\n%s\n", + len(moduleStatePaths), + strings.Join(moduleStatePathsSorted, "\n"), + ) // Collect all transitions from all modules - var allTransitions []StateTransition - for _, modulePath := range modulePaths { - stateFilePath := modulePath + "/state.json" - fmt.Fprintf(os.Stdout, "Analyzing %s...\n", stateFilePath) + var allTransitions []stateTransition + for _, moduleStatePath := range moduleStatePathsSorted { + fmt.Fprintf(os.Stdout, "Analyzing %s...\n", moduleStatePath) - transitions, err := GetStateFileTransitions(ctx, stateFilePath, baseRef, headRef) + transitions, err := getStateFileTransitions(ctx, moduleStatePath, baseRef, headRef) if err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to analyze %s: %v\n", stateFilePath, err) + fmt.Fprintf(os.Stderr, "Warning: failed to analyze %s: %v\n", moduleStatePath, err) continue } @@ -97,25 +110,23 @@ func run(ctx context.Context) error { } fmt.Fprintf(os.Stdout, "\nRunning casdiff for %d transition(s)...\n", len(allTransitions)) - - // Run casdiff for all transitions in parallel - results := RunCASDiffParallel(ctx, allTransitions) + results := runCASDiffs(ctx, allTransitions) // Separate successful results from errors var ( - successfulResults []CASDiffResult - failedResults []CASDiffResult + successfulResults []casDiffResult + failedResults []casDiffResult ) for _, result := range results { - if result.Error != nil { + if result.err != nil { failedResults = append(failedResults, result) fmt.Fprintf( - os.Stderr, "Warning: casdiff failed for %s %s->%s: %v\n", - result.Transition.ModulePath, - result.Transition.FromRef, - result.Transition.ToRef, - result.Error, + os.Stderr, "Warning: casdiff failed for %s %s->%s: %w\n", + result.transition.modulePath, + result.transition.fromRef, + result.transition.toRef, + result.err, ) } else { successfulResults = append(successfulResults, result) @@ -129,28 +140,28 @@ func run(ctx context.Context) error { fmt.Fprintf(os.Stdout, "\n[dry-run] %d comment(s) would be posted:\n", len(successfulResults)) for _, result := range successfulResults { fmt.Fprintf(os.Stdout, "\n--- %s (line %d: %s -> %s) ---\n%s\n", - result.Transition.FilePath, - result.Transition.LineNumber, - result.Transition.FromRef, - result.Transition.ToRef, - result.Output, + result.transition.filePath, + result.transition.lineNumber, + result.transition.fromRef, + result.transition.toRef, + result.output, ) } } else { fmt.Fprintf(os.Stdout, "\nPosting %d comment(s) to PR...\n", len(successfulResults)) - comments := make([]PRReviewComment, len(successfulResults)) + comments := make([]prReviewComment, len(successfulResults)) for i, result := range successfulResults { - comments[i] = PRReviewComment{ - PRNumber: prNumber, - FilePath: result.Transition.FilePath, - LineNumber: result.Transition.LineNumber, - Body: result.Output, - CommitID: headRef, + comments[i] = prReviewComment{ + prNumber: prNumber, + filePath: result.transition.filePath, + lineNumber: result.transition.lineNumber, + body: result.output, + commitID: headRef, } } - postErrs = PostReviewComments(ctx, comments...) + postErrs = postReviewComments(ctx, comments...) posted := 0 for _, err := range postErrs { if err != nil { @@ -168,9 +179,9 @@ func run(ctx context.Context) error { fmt.Fprintf(os.Stderr, "\nSummary: %d casdiff command(s) failed:\n", len(failedResults)) for _, result := range failedResults { fmt.Fprintf(os.Stderr, " - %s: %s -> %s\n", - result.Transition.ModulePath, - result.Transition.FromRef, - result.Transition.ToRef) + result.transition.modulePath, + result.transition.fromRef, + result.transition.toRef) } return fmt.Errorf("%d casdiff command(s) failed", len(failedResults)) } diff --git a/cmd/commentprcasdiff/module_finder.go b/cmd/commentprcasdiff/module_finder.go index 345b8fc1..fa97b56f 100644 --- a/cmd/commentprcasdiff/module_finder.go +++ b/cmd/commentprcasdiff/module_finder.go @@ -18,20 +18,12 @@ import ( "context" "fmt" "os/exec" - "path/filepath" "strings" - - "buf.build/go/standard/xslices" ) -// changedModuleStates returns module directories that had *any change* on its state.json files -// between the passed base and head refs. Returns paths like "modules/sync/bufbuild/protovalidate" -// (without /state.json suffix). -func changedModuleStates( - ctx context.Context, - baseRef string, - headRef string, -) ([]string, error) { +// changedModuleStateFiles returns modules' state files that had *any change* between the passed +// base and head refs. +func changedModuleStateFiles(ctx context.Context, baseRef string, headRef string) (map[string]struct{}, error) { // Get list of changed files in the PR cmd := exec.CommandContext(ctx, "git", "diff", "--name-only", baseRef, headRef) //nolint:gosec output, err := cmd.Output() @@ -39,20 +31,20 @@ func changedModuleStates( return nil, fmt.Errorf("git diff: %w", err) } - // Filter for state.json files in modules/sync/ - modulePaths := make(map[string]struct{}) + moduleStatePaths := make(map[string]struct{}) for line := range strings.SplitSeq(string(output), "\n") { line = strings.TrimSpace(line) if line == "" { continue } + if line == "modules/sync/state.json" { + continue // exclude the global modules' state.json + } // Look for "modules/sync///state.json" files if strings.HasPrefix(line, "modules/sync/") && strings.HasSuffix(line, "/state.json") { - // Extract module path (remove /state.json suffix) - modulePath := filepath.Dir(line) - modulePaths[modulePath] = struct{}{} + moduleStatePaths[line] = struct{}{} } } - return xslices.MapKeysToSortedSlice(modulePaths), nil + return moduleStatePaths, nil } diff --git a/cmd/commentprcasdiff/state_analyzer.go b/cmd/commentprcasdiff/state_analyzer.go index 16dcfb94..d3c9826d 100644 --- a/cmd/commentprcasdiff/state_analyzer.go +++ b/cmd/commentprcasdiff/state_analyzer.go @@ -24,17 +24,18 @@ import ( "strings" ) -// StateTransition represents a digest change in a module's state.json file. -type StateTransition struct { - ModulePath string // e.g., "modules/sync/bufbuild/protovalidate" - FilePath string // e.g., "modules/sync/bufbuild/protovalidate/state.json" - FromRef string // Last reference with old digest (e.g., "v1.1.0") - ToRef string // First reference with new digest (e.g., "v1.2.0") - FromDigest string // Old digest - ToDigest string // New digest - LineNumber int // Line in diff where new digest first appears +// stateTransition represents a digest change in a module's state.json file. +type stateTransition struct { + modulePath string // e.g., "modules/sync/bufbuild/protovalidate" + filePath string // e.g., "modules/sync/bufbuild/protovalidate/state.json" + fromRef string // Last reference with old digest (e.g., "v1.1.0") + toRef string // First reference with new digest (e.g., "v1.2.0") + fromDigest string // Old digest + toDigest string // New digest + lineNumber int // Line in diff where new digest first appears } +// FIXME: these references/state should use the proto schema. type moduleState struct { References []reference `json:"references"` } @@ -44,14 +45,14 @@ type reference struct { Digest string `json:"digest"` } -// GetStateFileTransitions reads state.json from base and head branches, -// compares the JSON arrays to find appended references, and detects digest transitions. -func GetStateFileTransitions( +// getStateFileTransitions reads state.json from base and head branches, compares the JSON arrays to +// find appended references, and detects digest transitions. +func getStateFileTransitions( ctx context.Context, filePath string, baseRef string, headRef string, -) ([]StateTransition, error) { +) ([]stateTransition, error) { // Read state.json from both branches baseContent, err := readFileAtRef(ctx, filePath, baseRef) if err != nil { @@ -73,23 +74,24 @@ func GetStateFileTransitions( } // Identify appended references - baseCount := len(baseState.References) - headCount := len(headState.References) - - if headCount <= baseCount { + // + // This only works for diffs that append references in the state file, not for diffs that + // update/remove existing refs in base. + baseRefsCount := len(baseState.References) + headRefsCount := len(headState.References) + if headRefsCount <= baseRefsCount { // No new references added return nil, nil } - - appendedRefs := headState.References[baseCount:] + appendedRefs := headState.References[baseRefsCount:] // Get the baseline digest (last reference in base state) var ( currentDigest string currentRef string ) - if baseCount > 0 { - lastBaseRef := baseState.References[baseCount-1] + if baseRefsCount > 0 { + lastBaseRef := baseState.References[baseRefsCount-1] currentDigest = lastBaseRef.Digest currentRef = lastBaseRef.Name } else if len(appendedRefs) > 0 { @@ -100,31 +102,30 @@ func GetStateFileTransitions( } // Get line number mapping for the appended references - lineNumbers, err := getLineNumbersForAppendedRefs(ctx, filePath, baseRef, headRef, baseCount, headCount) + lineNumbers, err := getLineNumbersForAppendedRefs(ctx, filePath, baseRef, headRef, baseRefsCount, headRefsCount) if err != nil { return nil, fmt.Errorf("get line numbers: %w", err) } // Detect digest transitions - var transitions []StateTransition + var transitions []stateTransition modulePath := filepath.Dir(filePath) - for i, ref := range appendedRefs { if ref.Digest != currentDigest { // Digest changed! Record transition - lineNumber := 0 + var lineNumber int if i < len(lineNumbers) { lineNumber = lineNumbers[i] } - transition := StateTransition{ - ModulePath: modulePath, - FilePath: filePath, - FromRef: currentRef, - ToRef: ref.Name, - FromDigest: currentDigest, - ToDigest: ref.Digest, - LineNumber: lineNumber, + transition := stateTransition{ + modulePath: modulePath, + filePath: filePath, + fromRef: currentRef, + toRef: ref.Name, + fromDigest: currentDigest, + toDigest: ref.Digest, + lineNumber: lineNumber, } transitions = append(transitions, transition) @@ -147,8 +148,8 @@ func readFileAtRef(ctx context.Context, filePath string, ref string) ([]byte, er return output, nil } -// getLineNumbersForAppendedRefs calculates the line numbers in the diff where each -// appended reference's digest appears. +// getLineNumbersForAppendedRefs calculates the line numbers in the diff where each appended +// reference's digest appears. func getLineNumbersForAppendedRefs( ctx context.Context, filePath string, @@ -167,8 +168,8 @@ func getLineNumbersForAppendedRefs( return parseLineNumbersFromDiff(string(output), headCount-baseCount) } -// parseLineNumbersFromDiff parses a git diff output and extracts line numbers -// where "digest" fields appear in added lines. +// parseLineNumbersFromDiff parses a git diff output and extracts line numbers where "digest" fields +// appear in added lines. func parseLineNumbersFromDiff(diffOutput string, expectedCount int) ([]int, error) { lineNumbers := make([]int, expectedCount) scanner := bufio.NewScanner(strings.NewReader(diffOutput)) From 35d09bd80c423b2893d1f7da885dc4850045a17b Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 11:18:03 -0500 Subject: [PATCH 15/37] Dedupe GH PR comments --- cmd/commentprcasdiff/comment_poster.go | 116 +++++++++++++++++++++---- cmd/commentprcasdiff/main.go | 2 +- 2 files changed, 99 insertions(+), 19 deletions(-) diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index 8b86e0f9..df774492 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -33,35 +33,119 @@ type prReviewComment struct { commitID string // Head commit SHA } +// existingReviewComment is a comment already posted on the PR. +type existingReviewComment struct { + ID int64 `json:"id"` + Path string `json:"path"` + Line int `json:"line"` + User struct { + Login string `json:"login"` + } `json:"user"` +} + +// commentKey uniquely identifies a comment by file and line. +type commentKey struct { + path string + line int +} + // postReviewComments posts review comments to specific lines in the PR diff. Uses GitHub API (via -// gh CLI) to create inline comments. +// gh CLI) to create inline comments. If a bot comment already exists at the same file/line, it is +// updated instead of creating a duplicate. func postReviewComments(ctx context.Context, comments ...prReviewComment) []error { - var errors []error + errs := make([]error, len(comments)) - // Get repository information repo, err := getRepositoryInfo(ctx) if err != nil { - for range comments { - errors = append(errors, fmt.Errorf("get repository info: %w", err)) + for i := range errs { + errs[i] = fmt.Errorf("get repository info: %w", err) } - return errors + return errs + } + + // Fetch existing bot comments once, keyed by (path, line). + existing, err := listExistingBotComments(ctx, repo, comments[0].prNumber) + if err != nil { + // Non-fatal: fall back to always creating new comments. + fmt.Fprintf(os.Stderr, "Warning: could not fetch existing comments, will create new ones: %v\n", err) + existing = map[commentKey]int64{} } - // Post each comment - for _, comment := range comments { - if err := postSingleReviewComment(ctx, repo, comment); err != nil { - errors = append(errors, err) + for i, comment := range comments { + key := commentKey{path: comment.filePath, line: comment.lineNumber} + if existingID, ok := existing[key]; ok { + errs[i] = updateReviewComment(ctx, repo, existingID, comment.body) } else { - errors = append(errors, nil) + errs[i] = postSingleReviewComment(ctx, repo, comment) + } + } + + return errs +} + +// listExistingBotComments returns a map of (path, line) → comment ID for all comments left by +// github-actions[bot] on the PR. +func listExistingBotComments(ctx context.Context, repo string, prNumber string) (map[commentKey]int64, error) { + cmd := exec.CommandContext( //nolint:gosec + ctx, + "gh", "api", + fmt.Sprintf("repos/%s/pulls/%s/comments?sort=created&direction=asc", repo, prNumber), + "--paginate", + ) + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("gh api list comments: %w (stderr: %s)", err, stderr.String()) + } + + var rawComments []existingReviewComment + if err := json.Unmarshal(stdout.Bytes(), &rawComments); err != nil { + return nil, fmt.Errorf("parse comments response: %w", err) + } + + result := make(map[commentKey]int64) + for _, c := range rawComments { + if c.User.Login == "github-actions[bot]" { + result[commentKey{path: c.Path, line: c.Line}] = c.ID } } + return result, nil +} + +// updateReviewComment updates the body of an existing review comment. +func updateReviewComment(ctx context.Context, repo string, commentID int64, body string) error { + requestBody := map[string]any{"body": body} + + requestJSON, err := json.Marshal(requestBody) + if err != nil { + return fmt.Errorf("marshal request: %w", err) + } - return errors + cmd := exec.CommandContext( //nolint:gosec + ctx, + "gh", "api", + fmt.Sprintf("repos/%s/pulls/comments/%d", repo, commentID), + "-X", "PATCH", + "--input", "-", + ) + + cmd.Stdin = bytes.NewReader(requestJSON) + + var stderr bytes.Buffer + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + return fmt.Errorf("gh api patch comment: %w (stderr: %s)", err, stderr.String()) + } + + return nil } // getRepositoryInfo gets the owner/repo from the current git repository. func getRepositoryInfo(ctx context.Context) (string, error) { - // Use gh CLI to get repository info cmd := exec.CommandContext(ctx, "gh", "repo", "view", "--json", "nameWithOwner", "-q", ".nameWithOwner") output, err := cmd.Output() if err != nil { @@ -72,7 +156,6 @@ func getRepositoryInfo(ctx context.Context) (string, error) { // postSingleReviewComment posts a single review comment using GitHub API. func postSingleReviewComment(ctx context.Context, repo string, comment prReviewComment) error { - // Prepare the API request body requestBody := map[string]any{ "body": comment.body, "commit_id": comment.commitID, @@ -85,8 +168,6 @@ func postSingleReviewComment(ctx context.Context, repo string, comment prReviewC return fmt.Errorf("marshal request: %w", err) } - // Use gh CLI to make the API request - // gh api repos/{owner}/{repo}/pulls/{pr_number}/comments -X POST --input - cmd := exec.CommandContext( //nolint:gosec ctx, "gh", "api", @@ -101,11 +182,10 @@ func postSingleReviewComment(ctx context.Context, repo string, comment prReviewC cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - // Check if GITHUB_TOKEN is set if os.Getenv("GITHUB_TOKEN") == "" { return errors.New("GITHUB_TOKEN environment variable not set") } - return fmt.Errorf("gh api failed: %w (stderr: %s)", err, stderr.String()) + return fmt.Errorf("gh api post comment: %w (stderr: %s)", err, stderr.String()) } return nil diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index 9fc7aef4..4feb3e6e 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -122,7 +122,7 @@ func run(ctx context.Context) error { if result.err != nil { failedResults = append(failedResults, result) fmt.Fprintf( - os.Stderr, "Warning: casdiff failed for %s %s->%s: %w\n", + os.Stderr, "Warning: casdiff failed for %s %s->%s: %v\n", result.transition.modulePath, result.transition.fromRef, result.transition.toRef, From 127c28b2e098dcb7122c0fbdddc9425bfb78c0a3 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 11:58:06 -0500 Subject: [PATCH 16/37] Manual nits --- cmd/commentprcasdiff/comment_poster.go | 96 ++++++++++++-------------- cmd/commentprcasdiff/main.go | 91 +++++++++++------------- 2 files changed, 83 insertions(+), 104 deletions(-) diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index df774492..d518ac51 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -20,17 +20,15 @@ import ( "encoding/json" "errors" "fmt" - "os" "os/exec" + "time" ) -// prReviewComment represents a comment to post on a specific line in a PR. +// prReviewComment represents a comment to be posted or patchedon a specific line in a PR. type prReviewComment struct { - prNumber string filePath string // File path in the PR (e.g., "modules/sync/bufbuild/protovalidate/state.json") lineNumber int // Line number in the diff body string // Comment body (casdiff output) - commitID string // Head commit SHA } // existingReviewComment is a comment already posted on the PR. @@ -52,74 +50,75 @@ type commentKey struct { // postReviewComments posts review comments to specific lines in the PR diff. Uses GitHub API (via // gh CLI) to create inline comments. If a bot comment already exists at the same file/line, it is // updated instead of creating a duplicate. -func postReviewComments(ctx context.Context, comments ...prReviewComment) []error { - errs := make([]error, len(comments)) - - repo, err := getRepositoryInfo(ctx) +func postReviewComments(ctx context.Context, prNumber uint, gitCommitID string, comments ...prReviewComment) error { + gitRepoOwnerAndName, err := getGitRepositoryOwnerAndName(ctx) if err != nil { - for i := range errs { - errs[i] = fmt.Errorf("get repository info: %w", err) - } - return errs + return fmt.Errorf("get repository owner/name: %w", err) } - // Fetch existing bot comments once, keyed by (path, line). - existing, err := listExistingBotComments(ctx, repo, comments[0].prNumber) + // Fetch existing pr comments bot comments, keyed by (path, line). + existingPRComments, err := listExistingBotComments(ctx, gitRepoOwnerAndName, prNumber) if err != nil { - // Non-fatal: fall back to always creating new comments. - fmt.Fprintf(os.Stderr, "Warning: could not fetch existing comments, will create new ones: %v\n", err) - existing = map[commentKey]int64{} + return fmt.Errorf("list existing bot comments: %w", err) } - for i, comment := range comments { + var errsPosting error + for _, comment := range comments { key := commentKey{path: comment.filePath, line: comment.lineNumber} - if existingID, ok := existing[key]; ok { - errs[i] = updateReviewComment(ctx, repo, existingID, comment.body) + if prCommentID, alreadyPosted := existingPRComments[key]; alreadyPosted { + if err := updateReviewComment(ctx, gitRepoOwnerAndName, prCommentID, comment.body); err != nil { + errsPosting = errors.Join(errsPosting, fmt.Errorf("updating existing comment in %v: %w", key, err)) + } } else { - errs[i] = postSingleReviewComment(ctx, repo, comment) + if err := postSingleReviewComment(ctx, gitRepoOwnerAndName, prNumber, gitCommitID, comment); err != nil { + errsPosting = errors.Join(errsPosting, fmt.Errorf("posting new comment comment in %v: %w", key, err)) + } } } - - return errs + return errsPosting } -// listExistingBotComments returns a map of (path, line) → comment ID for all comments left by +// listExistingBotComments returns a map of (path, line) → commentID for all comments left by // github-actions[bot] on the PR. -func listExistingBotComments(ctx context.Context, repo string, prNumber string) (map[commentKey]int64, error) { +func listExistingBotComments(ctx context.Context, repo string, prNumber uint) (map[commentKey]int64, error) { cmd := exec.CommandContext( //nolint:gosec ctx, "gh", "api", - fmt.Sprintf("repos/%s/pulls/%s/comments?sort=created&direction=asc", repo, prNumber), + fmt.Sprintf( + "repos/%s/pulls/%d/comments?"+ + "sort=created&direction=asc", // determinism: always patch the same comment id + repo, + prNumber, + ), "--paginate", ) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { return nil, fmt.Errorf("gh api list comments: %w (stderr: %s)", err, stderr.String()) } - var rawComments []existingReviewComment - if err := json.Unmarshal(stdout.Bytes(), &rawComments); err != nil { - return nil, fmt.Errorf("parse comments response: %w", err) + var existingComments []existingReviewComment + respBytes := stdout.Bytes() + if err := json.Unmarshal(respBytes, &existingComments); err != nil { + return nil, fmt.Errorf("parse comments response: %s: %w", string(respBytes), err) } result := make(map[commentKey]int64) - for _, c := range rawComments { - if c.User.Login == "github-actions[bot]" { - result[commentKey{path: c.Path, line: c.Line}] = c.ID + for _, comment := range existingComments { + if comment.User.Login == "github-actions[bot]" { + result[commentKey{path: comment.Path, line: comment.Line}] = comment.ID } } return result, nil } // updateReviewComment updates the body of an existing review comment. -func updateReviewComment(ctx context.Context, repo string, commentID int64, body string) error { - requestBody := map[string]any{"body": body} - - requestJSON, err := json.Marshal(requestBody) +func updateReviewComment(ctx context.Context, repoOwnerAndName string, commentID int64, body string) error { + commentBody := fmt.Sprintf("[Updated at %s]\n\n%s", time.Now(), body) + requestJSON, err := json.Marshal(map[string]any{"body": commentBody}) if err != nil { return fmt.Errorf("marshal request: %w", err) } @@ -127,25 +126,22 @@ func updateReviewComment(ctx context.Context, repo string, commentID int64, body cmd := exec.CommandContext( //nolint:gosec ctx, "gh", "api", - fmt.Sprintf("repos/%s/pulls/comments/%d", repo, commentID), + fmt.Sprintf("repos/%s/pulls/comments/%d", repoOwnerAndName, commentID), "-X", "PATCH", "--input", "-", ) - cmd.Stdin = bytes.NewReader(requestJSON) var stderr bytes.Buffer cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { return fmt.Errorf("gh api patch comment: %w (stderr: %s)", err, stderr.String()) } - return nil } -// getRepositoryInfo gets the owner/repo from the current git repository. -func getRepositoryInfo(ctx context.Context) (string, error) { +// getGitRepositoryOwnerAndName gets the owner/repo from the current git repository. +func getGitRepositoryOwnerAndName(ctx context.Context) (string, error) { cmd := exec.CommandContext(ctx, "gh", "repo", "view", "--json", "nameWithOwner", "-q", ".nameWithOwner") output, err := cmd.Output() if err != nil { @@ -155,12 +151,13 @@ func getRepositoryInfo(ctx context.Context) (string, error) { } // postSingleReviewComment posts a single review comment using GitHub API. -func postSingleReviewComment(ctx context.Context, repo string, comment prReviewComment) error { +func postSingleReviewComment(ctx context.Context, repoOwnerAndName string, prNumber uint, gitCommitID string, comment prReviewComment) error { + commentBody := fmt.Sprintf("[Posted at %s]\n\n%s", time.Now(), comment.body) requestBody := map[string]any{ - "body": comment.body, - "commit_id": comment.commitID, + "commit_id": gitCommitID, "path": comment.filePath, "line": comment.lineNumber, + "body": commentBody, } requestJSON, err := json.Marshal(requestBody) @@ -171,20 +168,15 @@ func postSingleReviewComment(ctx context.Context, repo string, comment prReviewC cmd := exec.CommandContext( //nolint:gosec ctx, "gh", "api", - fmt.Sprintf("repos/%s/pulls/%s/comments", repo, comment.prNumber), + fmt.Sprintf("repos/%s/pulls/%d/comments", repoOwnerAndName, prNumber), "-X", "POST", "--input", "-", ) - cmd.Stdin = bytes.NewReader(requestJSON) var stderr bytes.Buffer cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - if os.Getenv("GITHUB_TOKEN") == "" { - return errors.New("GITHUB_TOKEN environment variable not set") - } return fmt.Errorf("gh api post comment: %w (stderr: %s)", err, stderr.String()) } diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index 4feb3e6e..053295ee 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -21,6 +21,7 @@ import ( "flag" "fmt" "os" + "strconv" "strings" "buf.build/go/standard/xslices" @@ -39,24 +40,35 @@ func run(ctx context.Context) error { flag.Parse() // Read environment variables - prNumber := os.Getenv("PR_NUMBER") baseRef := os.Getenv("BASE_REF") headRef := os.Getenv("HEAD_REF") + prNumberString := os.Getenv("PR_NUMBER") + var prNumber uint64 - if prNumber == "" && !dryRun { - return errors.New("PR_NUMBER environment variable is required when not a dry-run") - } if baseRef == "" { return errors.New("BASE_REF environment variable is required") } if headRef == "" { return errors.New("HEAD_REF environment variable is required") } + if !dryRun { + if os.Getenv("GITHUB_TOKEN") == "" { + return errors.New("GITHUB_TOKEN environment variable is required when not a dry-run") + } + if prNumberString == "" { + return errors.New("PR_NUMBER environment variable is required when not a dry-run") + } + var err error + prNumber, err = strconv.ParseUint(prNumberString, 10, 64) + if err != nil { + return fmt.Errorf("parse PR_NUMBER: %w", err) + } + } fmt.Fprintf( os.Stdout, "Processing PR #%s (base: %s, head: %s)\n", - cmp.Or(prNumber, "dry-run"), + cmp.Or(prNumberString, "dry-run"), baseRef, headRef, ) @@ -112,33 +124,33 @@ func run(ctx context.Context) error { fmt.Fprintf(os.Stdout, "\nRunning casdiff for %d transition(s)...\n", len(allTransitions)) results := runCASDiffs(ctx, allTransitions) - // Separate successful results from errors var ( - successfulResults []casDiffResult - failedResults []casDiffResult + casResults []casDiffResult + errsToReturn []error ) for _, result := range results { if result.err != nil { - failedResults = append(failedResults, result) - fmt.Fprintf( - os.Stderr, "Warning: casdiff failed for %s %s->%s: %v\n", - result.transition.modulePath, - result.transition.fromRef, - result.transition.toRef, - result.err, + errsToReturn = append( + errsToReturn, + fmt.Errorf( + "casdiff failed for %s %s->%s: %w", + result.transition.modulePath, + result.transition.fromRef, + result.transition.toRef, + result.err, + ), ) } else { - successfulResults = append(successfulResults, result) + casResults = append(casResults, result) } } // Post or print comments for successful results - var postErrs []error - if len(successfulResults) > 0 { + if len(casResults) > 0 { if dryRun { - fmt.Fprintf(os.Stdout, "\n[dry-run] %d comment(s) would be posted:\n", len(successfulResults)) - for _, result := range successfulResults { + fmt.Fprintf(os.Stdout, "\n[dry-run] %d comment(s) would be posted:\n", len(casResults)) + for _, result := range casResults { fmt.Fprintf(os.Stdout, "\n--- %s (line %d: %s -> %s) ---\n%s\n", result.transition.filePath, result.transition.lineNumber, @@ -148,49 +160,24 @@ func run(ctx context.Context) error { ) } } else { - fmt.Fprintf(os.Stdout, "\nPosting %d comment(s) to PR...\n", len(successfulResults)) - - comments := make([]prReviewComment, len(successfulResults)) - for i, result := range successfulResults { + fmt.Fprintf(os.Stdout, "\nPosting %d comment(s) to PR...\n", len(casResults)) + comments := make([]prReviewComment, len(casResults)) + for i, result := range casResults { comments[i] = prReviewComment{ - prNumber: prNumber, filePath: result.transition.filePath, lineNumber: result.transition.lineNumber, body: result.output, - commitID: headRef, } } - - postErrs = postReviewComments(ctx, comments...) - posted := 0 - for _, err := range postErrs { - if err != nil { - fmt.Fprintf(os.Stderr, "Error: failed to post comment: %v\n", err) - } else { - posted++ - } + if err := postReviewComments(ctx, uint(prNumber), headRef, comments...); err != nil { + errsToReturn = append(errsToReturn, fmt.Errorf("post review comments: %w", err)) } - fmt.Fprintf(os.Stdout, "Successfully posted %d comment(s)\n", posted) } } - // Report failures and exit non-zero if anything went wrong. - if len(failedResults) > 0 { - fmt.Fprintf(os.Stderr, "\nSummary: %d casdiff command(s) failed:\n", len(failedResults)) - for _, result := range failedResults { - fmt.Fprintf(os.Stderr, " - %s: %s -> %s\n", - result.transition.modulePath, - result.transition.fromRef, - result.transition.toRef) - } - return fmt.Errorf("%d casdiff command(s) failed", len(failedResults)) + if len(errsToReturn) > 0 { + return errors.Join(errsToReturn...) } - for _, err := range postErrs { - if err != nil { - return fmt.Errorf("one or more comments failed to post") - } - } - fmt.Fprintf(os.Stdout, "\nDone!\n") return nil } From 9dc5c6dbacdf06e025e94fb57cb91f8296c73b64 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 12:02:56 -0500 Subject: [PATCH 17/37] time format --- cmd/commentprcasdiff/comment_poster.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index d518ac51..125afa91 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -117,7 +117,7 @@ func listExistingBotComments(ctx context.Context, repo string, prNumber uint) (m // updateReviewComment updates the body of an existing review comment. func updateReviewComment(ctx context.Context, repoOwnerAndName string, commentID int64, body string) error { - commentBody := fmt.Sprintf("[Updated at %s]\n\n%s", time.Now(), body) + commentBody := fmt.Sprintf("[Updated at %s]\n\n%s", time.Now().Format(time.RFC3339), body) requestJSON, err := json.Marshal(map[string]any{"body": commentBody}) if err != nil { return fmt.Errorf("marshal request: %w", err) @@ -152,7 +152,7 @@ func getGitRepositoryOwnerAndName(ctx context.Context) (string, error) { // postSingleReviewComment posts a single review comment using GitHub API. func postSingleReviewComment(ctx context.Context, repoOwnerAndName string, prNumber uint, gitCommitID string, comment prReviewComment) error { - commentBody := fmt.Sprintf("[Posted at %s]\n\n%s", time.Now(), comment.body) + commentBody := fmt.Sprintf("[Posted at %s]\n\n%s", time.Now().Format(time.RFC3339), comment.body) requestBody := map[string]any{ "commit_id": gitCommitID, "path": comment.filePath, From 425d58bd7d92bf2d485a519262c68d3ba9585b79 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 12:14:08 -0500 Subject: [PATCH 18/37] Reorganize comments poster --- cmd/commentprcasdiff/comment_poster.go | 82 +++++++++++++------------- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index 125afa91..2221f234 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -24,7 +24,7 @@ import ( "time" ) -// prReviewComment represents a comment to be posted or patchedon a specific line in a PR. +// prReviewComment represents a comment to be posted or patched on a specific line in a PR. type prReviewComment struct { filePath string // File path in the PR (e.g., "modules/sync/bufbuild/protovalidate/state.json") lineNumber int // Line number in the diff @@ -62,20 +62,33 @@ func postReviewComments(ctx context.Context, prNumber uint, gitCommitID string, return fmt.Errorf("list existing bot comments: %w", err) } - var errsPosting error + var errsPosting []error for _, comment := range comments { key := commentKey{path: comment.filePath, line: comment.lineNumber} if prCommentID, alreadyPosted := existingPRComments[key]; alreadyPosted { if err := updateReviewComment(ctx, gitRepoOwnerAndName, prCommentID, comment.body); err != nil { - errsPosting = errors.Join(errsPosting, fmt.Errorf("updating existing comment in %v: %w", key, err)) + errsPosting = append(errsPosting, fmt.Errorf("updating existing comment in %v: %w", key, err)) } } else { if err := postSingleReviewComment(ctx, gitRepoOwnerAndName, prNumber, gitCommitID, comment); err != nil { - errsPosting = errors.Join(errsPosting, fmt.Errorf("posting new comment comment in %v: %w", key, err)) + errsPosting = append(errsPosting, fmt.Errorf("posting new comment in %v: %w", key, err)) } } } - return errsPosting + if len(errsPosting) > 0 { + return errors.Join(errsPosting...) + } + return nil +} + +// getGitRepositoryOwnerAndName gets the owner/repo from the current git repository. +func getGitRepositoryOwnerAndName(ctx context.Context) (string, error) { + cmd := exec.CommandContext(ctx, "gh", "repo", "view", "--json", "nameWithOwner", "-q", ".nameWithOwner") + output, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("gh repo view: %w", err) + } + return string(bytes.TrimSpace(output)), nil } // listExistingBotComments returns a map of (path, line) → commentID for all comments left by @@ -106,79 +119,64 @@ func listExistingBotComments(ctx context.Context, repo string, prNumber uint) (m return nil, fmt.Errorf("parse comments response: %s: %w", string(respBytes), err) } + const githubActionsBotUsername = "github-actions[bot]" result := make(map[commentKey]int64) for _, comment := range existingComments { - if comment.User.Login == "github-actions[bot]" { + if comment.User.Login == githubActionsBotUsername { result[commentKey{path: comment.Path, line: comment.Line}] = comment.ID } } return result, nil } -// updateReviewComment updates the body of an existing review comment. -func updateReviewComment(ctx context.Context, repoOwnerAndName string, commentID int64, body string) error { - commentBody := fmt.Sprintf("[Updated at %s]\n\n%s", time.Now().Format(time.RFC3339), body) - requestJSON, err := json.Marshal(map[string]any{"body": commentBody}) +func postSingleReviewComment(ctx context.Context, repoOwnerAndName string, prNumber uint, gitCommitID string, comment prReviewComment) error { + commentBody := fmt.Sprintf("_[Posted at %s]_\n\n%s", time.Now().Format(time.RFC3339), comment.body) + requestBody := map[string]any{ + "commit_id": gitCommitID, + "path": comment.filePath, + "line": comment.lineNumber, + "body": commentBody, + } + requestJSON, err := json.Marshal(requestBody) if err != nil { return fmt.Errorf("marshal request: %w", err) } + var stderr bytes.Buffer cmd := exec.CommandContext( //nolint:gosec ctx, "gh", "api", - fmt.Sprintf("repos/%s/pulls/comments/%d", repoOwnerAndName, commentID), - "-X", "PATCH", + fmt.Sprintf("repos/%s/pulls/%d/comments", repoOwnerAndName, prNumber), + "-X", "POST", "--input", "-", ) cmd.Stdin = bytes.NewReader(requestJSON) - - var stderr bytes.Buffer cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - return fmt.Errorf("gh api patch comment: %w (stderr: %s)", err, stderr.String()) + return fmt.Errorf("gh api post comment: %w (stderr: %s)", err, stderr.String()) } return nil } -// getGitRepositoryOwnerAndName gets the owner/repo from the current git repository. -func getGitRepositoryOwnerAndName(ctx context.Context) (string, error) { - cmd := exec.CommandContext(ctx, "gh", "repo", "view", "--json", "nameWithOwner", "-q", ".nameWithOwner") - output, err := cmd.Output() - if err != nil { - return "", fmt.Errorf("gh repo view: %w", err) - } - return string(bytes.TrimSpace(output)), nil -} - -// postSingleReviewComment posts a single review comment using GitHub API. -func postSingleReviewComment(ctx context.Context, repoOwnerAndName string, prNumber uint, gitCommitID string, comment prReviewComment) error { - commentBody := fmt.Sprintf("[Posted at %s]\n\n%s", time.Now().Format(time.RFC3339), comment.body) - requestBody := map[string]any{ - "commit_id": gitCommitID, - "path": comment.filePath, - "line": comment.lineNumber, - "body": commentBody, - } - - requestJSON, err := json.Marshal(requestBody) +func updateReviewComment(ctx context.Context, repoOwnerAndName string, prCommentID int64, body string) error { + commentBody := fmt.Sprintf("_[Updated at %s]_\n\n%s", time.Now().Format(time.RFC3339), body) + requestJSON, err := json.Marshal(map[string]any{"body": commentBody}) if err != nil { return fmt.Errorf("marshal request: %w", err) } + var stderr bytes.Buffer cmd := exec.CommandContext( //nolint:gosec ctx, "gh", "api", - fmt.Sprintf("repos/%s/pulls/%d/comments", repoOwnerAndName, prNumber), - "-X", "POST", + fmt.Sprintf("repos/%s/pulls/comments/%d", repoOwnerAndName, prCommentID), + "-X", "PATCH", "--input", "-", ) cmd.Stdin = bytes.NewReader(requestJSON) - - var stderr bytes.Buffer cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - return fmt.Errorf("gh api post comment: %w (stderr: %s)", err, stderr.String()) + return fmt.Errorf("gh api patch comment: %w (stderr: %s)", err, stderr.String()) } - return nil } From 20300536684b9635d7984d0a30537e9e8984a06e Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 12:28:46 -0500 Subject: [PATCH 19/37] Use proto schema --- cmd/commentprcasdiff/main.go | 8 ++- cmd/commentprcasdiff/state_analyzer.go | 67 +++++++++++--------------- 2 files changed, 36 insertions(+), 39 deletions(-) diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index 053295ee..b1b7ef17 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -25,6 +25,7 @@ import ( "strings" "buf.build/go/standard/xslices" + "github.com/bufbuild/modules/private/bufpkg/bufstate" ) func main() { @@ -97,12 +98,17 @@ func run(ctx context.Context) error { strings.Join(moduleStatePathsSorted, "\n"), ) + stateRW, err := bufstate.NewReadWriter() + if err != nil { + return fmt.Errorf("new state read writer: %w", err) + } + // Collect all transitions from all modules var allTransitions []stateTransition for _, moduleStatePath := range moduleStatePathsSorted { fmt.Fprintf(os.Stdout, "Analyzing %s...\n", moduleStatePath) - transitions, err := getStateFileTransitions(ctx, moduleStatePath, baseRef, headRef) + transitions, err := getStateFileTransitions(ctx, stateRW, moduleStatePath, baseRef, headRef) if err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to analyze %s: %v\n", moduleStatePath, err) continue diff --git a/cmd/commentprcasdiff/state_analyzer.go b/cmd/commentprcasdiff/state_analyzer.go index d3c9826d..49391df9 100644 --- a/cmd/commentprcasdiff/state_analyzer.go +++ b/cmd/commentprcasdiff/state_analyzer.go @@ -16,12 +16,15 @@ package main import ( "bufio" + "bytes" "context" - "encoding/json" "fmt" + "io" "os/exec" "path/filepath" "strings" + + "github.com/bufbuild/modules/private/bufpkg/bufstate" ) // stateTransition represents a digest change in a module's state.json file. @@ -35,20 +38,11 @@ type stateTransition struct { lineNumber int // Line in diff where new digest first appears } -// FIXME: these references/state should use the proto schema. -type moduleState struct { - References []reference `json:"references"` -} - -type reference struct { - Name string `json:"name"` - Digest string `json:"digest"` -} - // getStateFileTransitions reads state.json from base and head branches, compares the JSON arrays to // find appended references, and detects digest transitions. func getStateFileTransitions( ctx context.Context, + stateRW *bufstate.ReadWriter, filePath string, baseRef string, headRef string, @@ -58,32 +52,33 @@ func getStateFileTransitions( if err != nil { return nil, fmt.Errorf("read base state: %w", err) } - headContent, err := readFileAtRef(ctx, filePath, headRef) if err != nil { return nil, fmt.Errorf("read head state: %w", err) } - // Parse JSON - var baseState, headState moduleState - if err := json.Unmarshal(baseContent, &baseState); err != nil { - return nil, fmt.Errorf("parse base state JSON: %w", err) + baseState, err := stateRW.ReadModStateFile(io.NopCloser(bytes.NewReader(baseContent))) + if err != nil { + return nil, fmt.Errorf("parse base state: %w", err) } - if err := json.Unmarshal(headContent, &headState); err != nil { - return nil, fmt.Errorf("parse head state JSON: %w", err) + headState, err := stateRW.ReadModStateFile(io.NopCloser(bytes.NewReader(headContent))) + if err != nil { + return nil, fmt.Errorf("parse head state: %w", err) } - // Identify appended references + // Identify appended references. // // This only works for diffs that append references in the state file, not for diffs that // update/remove existing refs in base. - baseRefsCount := len(baseState.References) - headRefsCount := len(headState.References) + baseRefs := baseState.GetReferences() + headRefs := headState.GetReferences() + baseRefsCount := len(baseRefs) + headRefsCount := len(headRefs) if headRefsCount <= baseRefsCount { // No new references added return nil, nil } - appendedRefs := headState.References[baseRefsCount:] + appendedRefs := headRefs[baseRefsCount:] // Get the baseline digest (last reference in base state) var ( @@ -91,13 +86,13 @@ func getStateFileTransitions( currentRef string ) if baseRefsCount > 0 { - lastBaseRef := baseState.References[baseRefsCount-1] - currentDigest = lastBaseRef.Digest - currentRef = lastBaseRef.Name + lastBaseRef := baseRefs[baseRefsCount-1] + currentDigest = lastBaseRef.GetDigest() + currentRef = lastBaseRef.GetName() } else if len(appendedRefs) > 0 { // If base state is empty, use first appended ref as baseline - currentDigest = appendedRefs[0].Digest - currentRef = appendedRefs[0].Name + currentDigest = appendedRefs[0].GetDigest() + currentRef = appendedRefs[0].GetName() appendedRefs = appendedRefs[1:] // Skip first as it's now the baseline } @@ -111,28 +106,24 @@ func getStateFileTransitions( var transitions []stateTransition modulePath := filepath.Dir(filePath) for i, ref := range appendedRefs { - if ref.Digest != currentDigest { + if ref.GetDigest() != currentDigest { // Digest changed! Record transition var lineNumber int if i < len(lineNumbers) { lineNumber = lineNumbers[i] } - - transition := stateTransition{ + transitions = append(transitions, stateTransition{ modulePath: modulePath, filePath: filePath, fromRef: currentRef, - toRef: ref.Name, + toRef: ref.GetName(), fromDigest: currentDigest, - toDigest: ref.Digest, + toDigest: ref.GetDigest(), lineNumber: lineNumber, - } - transitions = append(transitions, transition) - - // Update current for next iteration - currentDigest = ref.Digest + }) + currentDigest = ref.GetDigest() } - currentRef = ref.Name + currentRef = ref.GetName() } return transitions, nil From 4842e4693fe30d2a96514afc868589c4677cbad8 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 13:45:25 -0500 Subject: [PATCH 20/37] Test and comment nits --- cmd/commentprcasdiff/casdiff_runner.go | 7 ++++- cmd/commentprcasdiff/state_analyzer.go | 14 ++------- cmd/commentprcasdiff/state_analyzer_test.go | 34 ++++++++++++--------- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/cmd/commentprcasdiff/casdiff_runner.go b/cmd/commentprcasdiff/casdiff_runner.go index c5ef0b2c..cad08a89 100644 --- a/cmd/commentprcasdiff/casdiff_runner.go +++ b/cmd/commentprcasdiff/casdiff_runner.go @@ -64,7 +64,12 @@ func runCASDiff(ctx context.Context, transition stateTransition) casDiffResult { return result } - result.output = stdout.String() + result.output = fmt.Sprintf( + "```sh\n$ casdiff %s %s --format=markdown\n```\n\n%s", + transition.fromRef, + transition.toRef, + stdout.String(), + ) return result } diff --git a/cmd/commentprcasdiff/state_analyzer.go b/cmd/commentprcasdiff/state_analyzer.go index 49391df9..0f3d1ca0 100644 --- a/cmd/commentprcasdiff/state_analyzer.go +++ b/cmd/commentprcasdiff/state_analyzer.go @@ -149,13 +149,12 @@ func getLineNumbersForAppendedRefs( baseCount int, headCount int, ) ([]int, error) { - // Get the unified diff + // Get the unified diff, with zero lines of context up or down. cmd := exec.CommandContext(ctx, "git", "diff", "-U0", baseRef, headRef, "--", filePath) //nolint:gosec output, err := cmd.Output() if err != nil { return nil, fmt.Errorf("git diff: %w", err) } - return parseLineNumbersFromDiff(string(output), headCount-baseCount) } @@ -170,10 +169,8 @@ func parseLineNumbersFromDiff(diffOutput string, expectedCount int) ([]int, erro refIndex = 0 inAddedSection = false ) - for scanner.Scan() { line := scanner.Text() - // Parse hunk headers like: @@ -275,0 +276,12 @@ if strings.HasPrefix(line, "@@") { parts := strings.Split(line, " ") @@ -188,15 +185,12 @@ func parseLineNumbersFromDiff(diffOutput string, expectedCount int) ([]int, erro continue } } - if !inAddedSection { continue } - - // Look for added lines (starting with +) + // Look for added lines (starting with +) with the string `"digest":` if strings.HasPrefix(line, "+") && !strings.HasPrefix(line, "+++") { - // Check if this line contains "digest" - if strings.Contains(line, `"digest"`) { + if strings.Contains(line, `"digest":`) { if refIndex < len(lineNumbers) { lineNumbers[refIndex] = currentLine refIndex++ @@ -205,10 +199,8 @@ func parseLineNumbersFromDiff(diffOutput string, expectedCount int) ([]int, erro currentLine++ } } - if err := scanner.Err(); err != nil { return nil, fmt.Errorf("scan diff: %w", err) } - return lineNumbers, nil } diff --git a/cmd/commentprcasdiff/state_analyzer_test.go b/cmd/commentprcasdiff/state_analyzer_test.go index c6719806..f072a930 100644 --- a/cmd/commentprcasdiff/state_analyzer_test.go +++ b/cmd/commentprcasdiff/state_analyzer_test.go @@ -21,7 +21,7 @@ import ( func TestParseLineNumbersFromDiff(t *testing.T) { t.Parallel() - tests := []struct { + testCases := []struct { name string diffOutput string expectedCount int @@ -34,7 +34,7 @@ func TestParseLineNumbersFromDiff(t *testing.T) { + }, + { + "name": "v1.1.0", -+ "digest": "35b3d88f6b0fbf159d9eedfc2fbfa976490e6bca1d98914c1c71f29bfe2da6261fca56c057975b3e5c022b3234a0f2eea8e2d1b599a937c6c5d63d21201a9bc3" ++ "digest": "aaa" + } + ]`, expectedCount: 1, @@ -65,13 +65,17 @@ func TestParseLineNumbersFromDiff(t *testing.T) { + "name": "v2.0.0", + "digest": "xxx" + } -@@ -100,0 +105,4 @@ +@@ -100,0 +105,8 @@ + { + "name": "v3.0.0", + "digest": "yyy" ++ }, ++ { ++ "name": "v4.0.0", ++ "digest": "zzz" + }`, - expectedCount: 2, - want: []int{53, 107}, + expectedCount: 3, + want: []int{53, 107, 111}, }, { name: "no_digest_lines", @@ -90,22 +94,22 @@ func TestParseLineNumbersFromDiff(t *testing.T) { want: []int{0}, }, { - name: "digest_with_spaces_and_formatting", + name: "digest_with_formatting", diffOutput: `@@ -200,0 +201,8 @@ -+ { -+ "name": "v0.1.0", -+ "digest": "abc123" -+ }, -+ { -+ "name": "v0.2.0", -+ "digest": "def456" -+ }`, ++{ ++"name": "v0.1.0", ++"digest": "abc123" ++}, ++ { ++ "name": "v0.2.0", ++ "digest": "def456" ++ }`, expectedCount: 2, want: []int{203, 207}, }, } - for _, tt := range tests { + for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { t.Parallel() got, err := parseLineNumbersFromDiff(tt.diffOutput, tt.expectedCount) From 2edf93b7bf16bbf0de4c345f1323bb2bdd9a7e6f Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 13:59:15 -0500 Subject: [PATCH 21/37] lint --- .golangci.yaml | 1 + cmd/commentprcasdiff/state_analyzer_test.go | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 5f17f0ec..720b2e12 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -37,6 +37,7 @@ linters: - T any - i int - wg sync.WaitGroup + - tc testCase exclusions: generated: lax presets: diff --git a/cmd/commentprcasdiff/state_analyzer_test.go b/cmd/commentprcasdiff/state_analyzer_test.go index f072a930..736eb44f 100644 --- a/cmd/commentprcasdiff/state_analyzer_test.go +++ b/cmd/commentprcasdiff/state_analyzer_test.go @@ -21,13 +21,14 @@ import ( func TestParseLineNumbersFromDiff(t *testing.T) { t.Parallel() - testCases := []struct { + type testCase struct { name string diffOutput string expectedCount int want []int wantErr bool - }{ + } + testCases := []testCase{ { name: "single_digest_change", diffOutput: `@@ -275,0 +276,6 @@ @@ -109,16 +110,16 @@ func TestParseLineNumbersFromDiff(t *testing.T) { }, } - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, err := parseLineNumbersFromDiff(tt.diffOutput, tt.expectedCount) - if (err != nil) != tt.wantErr { - t.Errorf("parseLineNumbersFromDiff() error = %v, wantErr %v", err, tt.wantErr) + got, err := parseLineNumbersFromDiff(tc.diffOutput, tc.expectedCount) + if (err != nil) != tc.wantErr { + t.Errorf("parseLineNumbersFromDiff() error = %v, wantErr %v", err, tc.wantErr) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("parseLineNumbersFromDiff() = %v, want %v", got, tt.want) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("parseLineNumbersFromDiff() = %v, want %v", got, tc.want) } }) } From a41d8fc6c51d171a2e7d715fd38f60030d254a6d Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 13:59:32 -0500 Subject: [PATCH 22/37] Reapply "Remove some state changes" This reverts commit 610b51bc629160612bd59b548038a54d7e7efe73. --- .../protocolbuffers/wellknowntypes/state.json | 26 +------------------ modules/sync/state.json | 4 +-- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/modules/sync/protocolbuffers/wellknowntypes/state.json b/modules/sync/protocolbuffers/wellknowntypes/state.json index ddbce932..a6ba7303 100644 --- a/modules/sync/protocolbuffers/wellknowntypes/state.json +++ b/modules/sync/protocolbuffers/wellknowntypes/state.json @@ -463,30 +463,6 @@ { "name": "v33.0", "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" - }, - { - "name": "v33.1", - "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" - }, - { - "name": "v33.2", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.3", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.4", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v33.5", - "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" - }, - { - "name": "v34.0", - "digest": "946278992baee216f7742401780df3727195ffc068a30e21769c7e808d16ee25e9e48e51f938b6ddbe405abe46c45091a56ad90aac1cad0d809d16cbad889ff1" } ] -} \ No newline at end of file +} diff --git a/modules/sync/state.json b/modules/sync/state.json index b3d5424d..b6c34896 100644 --- a/modules/sync/state.json +++ b/modules/sync/state.json @@ -78,7 +78,7 @@ }, { "module_name": "protocolbuffers/wellknowntypes", - "latest_reference": "v34.0" + "latest_reference": "v33.0" } ] -} \ No newline at end of file +} From faedd0c445d63b7bde1e085592cc20c448b0af62 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 14:00:27 -0500 Subject: [PATCH 23/37] Enable workflow --- .github/workflows/auto-casdiff-comment.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/auto-casdiff-comment.yaml b/.github/workflows/auto-casdiff-comment.yaml index 017948c2..7ab212ca 100644 --- a/.github/workflows/auto-casdiff-comment.yaml +++ b/.github/workflows/auto-casdiff-comment.yaml @@ -3,9 +3,8 @@ name: Auto-comment PR with casdiff on: pull_request: types: [opened, synchronize] - # FIXME: re-enable once tested - # branches: - # - main + branches: + - main permissions: contents: read From 7eb6345c5069a72ddfa2aad73b0dc4c81d0ba43e Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 14:04:04 -0500 Subject: [PATCH 24/37] Revert "Reapply "Remove some state changes"" This reverts commit a41d8fc6c51d171a2e7d715fd38f60030d254a6d. --- .../protocolbuffers/wellknowntypes/state.json | 26 ++++++++++++++++++- modules/sync/state.json | 4 +-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/modules/sync/protocolbuffers/wellknowntypes/state.json b/modules/sync/protocolbuffers/wellknowntypes/state.json index a6ba7303..ddbce932 100644 --- a/modules/sync/protocolbuffers/wellknowntypes/state.json +++ b/modules/sync/protocolbuffers/wellknowntypes/state.json @@ -463,6 +463,30 @@ { "name": "v33.0", "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" + }, + { + "name": "v33.1", + "digest": "49b3059e6608c257ea7cf60926a16fb8bb1f3d37f39862e66db55338a4ebf59a4aebff39fdfd1f6d4e66ece567db327ff5846a09b51762574b857a27e77a2b55" + }, + { + "name": "v33.2", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.3", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.4", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v33.5", + "digest": "7535ac337929b4cfdd12d52bef75155277e715bdc364fcb41556a95fa0d3f58510b4e210f609cb91f9a47363a59c1643b6b1059d7702d0e900059271fde1c03a" + }, + { + "name": "v34.0", + "digest": "946278992baee216f7742401780df3727195ffc068a30e21769c7e808d16ee25e9e48e51f938b6ddbe405abe46c45091a56ad90aac1cad0d809d16cbad889ff1" } ] -} +} \ No newline at end of file diff --git a/modules/sync/state.json b/modules/sync/state.json index b6c34896..b3d5424d 100644 --- a/modules/sync/state.json +++ b/modules/sync/state.json @@ -78,7 +78,7 @@ }, { "module_name": "protocolbuffers/wellknowntypes", - "latest_reference": "v33.0" + "latest_reference": "v34.0" } ] -} +} \ No newline at end of file From e19a2e89039a5a1749ca5ca326b37b9ca6f6104c Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 14:05:24 -0500 Subject: [PATCH 25/37] Simplify test --- cmd/commentprcasdiff/state_analyzer_test.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/cmd/commentprcasdiff/state_analyzer_test.go b/cmd/commentprcasdiff/state_analyzer_test.go index 736eb44f..51b937b1 100644 --- a/cmd/commentprcasdiff/state_analyzer_test.go +++ b/cmd/commentprcasdiff/state_analyzer_test.go @@ -15,8 +15,10 @@ package main import ( - "reflect" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestParseLineNumbersFromDiff(t *testing.T) { @@ -114,13 +116,8 @@ func TestParseLineNumbersFromDiff(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() got, err := parseLineNumbersFromDiff(tc.diffOutput, tc.expectedCount) - if (err != nil) != tc.wantErr { - t.Errorf("parseLineNumbersFromDiff() error = %v, wantErr %v", err, tc.wantErr) - return - } - if !reflect.DeepEqual(got, tc.want) { - t.Errorf("parseLineNumbersFromDiff() = %v, want %v", got, tc.want) - } + require.NoError(t, err) + assert.Equal(t, tc.want, got) }) } } From 2c57e463d52e35ec8e1cd4932f7fb3b5bc7ad6c8 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 14:07:06 -0500 Subject: [PATCH 26/37] Paths filter --- .github/workflows/auto-casdiff-comment.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/auto-casdiff-comment.yaml b/.github/workflows/auto-casdiff-comment.yaml index 7ab212ca..96e83864 100644 --- a/.github/workflows/auto-casdiff-comment.yaml +++ b/.github/workflows/auto-casdiff-comment.yaml @@ -5,6 +5,8 @@ on: types: [opened, synchronize] branches: - main + paths: + - modules/sync/*/*/state.json permissions: contents: read From ebd2529956d1d6a7741b75df6550446fd9aa2f3d Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 14:10:04 -0500 Subject: [PATCH 27/37] lint --- cmd/commentprcasdiff/state_analyzer_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/commentprcasdiff/state_analyzer_test.go b/cmd/commentprcasdiff/state_analyzer_test.go index 51b937b1..252d6e35 100644 --- a/cmd/commentprcasdiff/state_analyzer_test.go +++ b/cmd/commentprcasdiff/state_analyzer_test.go @@ -28,7 +28,6 @@ func TestParseLineNumbersFromDiff(t *testing.T) { diffOutput string expectedCount int want []int - wantErr bool } testCases := []testCase{ { From b145d818546a8f8ff1b0f9a35694bae3fbef348c Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 16:25:55 -0500 Subject: [PATCH 28/37] Use wg.Go --- cmd/commentprcasdiff/casdiff_runner.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cmd/commentprcasdiff/casdiff_runner.go b/cmd/commentprcasdiff/casdiff_runner.go index cad08a89..af666ac1 100644 --- a/cmd/commentprcasdiff/casdiff_runner.go +++ b/cmd/commentprcasdiff/casdiff_runner.go @@ -79,11 +79,9 @@ func runCASDiffs(ctx context.Context, transitions []stateTransition) []casDiffRe var wg sync.WaitGroup for i, transition := range transitions { - wg.Add(1) - go func(index int, trans stateTransition) { - defer wg.Done() - results[index] = runCASDiff(ctx, trans) - }(i, transition) + wg.Go(func() { + results[i] = runCASDiff(ctx, transition) + }) } wg.Wait() From 60c54f0cfecb7eb6f609455aeca68f51ab416eee Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 16:35:40 -0500 Subject: [PATCH 29/37] Use gh dependency instead of shelling out to gh CLI --- cmd/commentprcasdiff/comment_poster.go | 171 +++++++++---------------- 1 file changed, 63 insertions(+), 108 deletions(-) diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index 2221f234..91a22b07 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -15,13 +15,13 @@ package main import ( - "bytes" "context" - "encoding/json" "errors" "fmt" - "os/exec" "time" + + "github.com/bufbuild/modules/internal/githubutil" + "github.com/google/go-github/v64/github" ) // prReviewComment represents a comment to be posted or patched on a specific line in a PR. @@ -31,33 +31,18 @@ type prReviewComment struct { body string // Comment body (casdiff output) } -// existingReviewComment is a comment already posted on the PR. -type existingReviewComment struct { - ID int64 `json:"id"` - Path string `json:"path"` - Line int `json:"line"` - User struct { - Login string `json:"login"` - } `json:"user"` -} - // commentKey uniquely identifies a comment by file and line. type commentKey struct { path string line int } -// postReviewComments posts review comments to specific lines in the PR diff. Uses GitHub API (via -// gh CLI) to create inline comments. If a bot comment already exists at the same file/line, it is -// updated instead of creating a duplicate. +// postReviewComments posts review comments to specific lines in the PR diff. If a bot comment +// already exists at the same file/line, it is updated instead of creating a duplicate. func postReviewComments(ctx context.Context, prNumber uint, gitCommitID string, comments ...prReviewComment) error { - gitRepoOwnerAndName, err := getGitRepositoryOwnerAndName(ctx) - if err != nil { - return fmt.Errorf("get repository owner/name: %w", err) - } + client := githubutil.NewClient(ctx) - // Fetch existing pr comments bot comments, keyed by (path, line). - existingPRComments, err := listExistingBotComments(ctx, gitRepoOwnerAndName, prNumber) + existingPRComments, err := listExistingBotComments(ctx, client, int(prNumber)) if err != nil { return fmt.Errorf("list existing bot comments: %w", err) } @@ -66,117 +51,87 @@ func postReviewComments(ctx context.Context, prNumber uint, gitCommitID string, for _, comment := range comments { key := commentKey{path: comment.filePath, line: comment.lineNumber} if prCommentID, alreadyPosted := existingPRComments[key]; alreadyPosted { - if err := updateReviewComment(ctx, gitRepoOwnerAndName, prCommentID, comment.body); err != nil { + if err := updateReviewComment(ctx, client, prCommentID, comment.body); err != nil { errsPosting = append(errsPosting, fmt.Errorf("updating existing comment in %v: %w", key, err)) } } else { - if err := postSingleReviewComment(ctx, gitRepoOwnerAndName, prNumber, gitCommitID, comment); err != nil { + if err := postSingleReviewComment(ctx, client, int(prNumber), gitCommitID, comment); err != nil { errsPosting = append(errsPosting, fmt.Errorf("posting new comment in %v: %w", key, err)) } } } - if len(errsPosting) > 0 { - return errors.Join(errsPosting...) - } - return nil -} - -// getGitRepositoryOwnerAndName gets the owner/repo from the current git repository. -func getGitRepositoryOwnerAndName(ctx context.Context) (string, error) { - cmd := exec.CommandContext(ctx, "gh", "repo", "view", "--json", "nameWithOwner", "-q", ".nameWithOwner") - output, err := cmd.Output() - if err != nil { - return "", fmt.Errorf("gh repo view: %w", err) - } - return string(bytes.TrimSpace(output)), nil + return errors.Join(errsPosting...) } // listExistingBotComments returns a map of (path, line) → commentID for all comments left by -// github-actions[bot] on the PR. -func listExistingBotComments(ctx context.Context, repo string, prNumber uint) (map[commentKey]int64, error) { - cmd := exec.CommandContext( //nolint:gosec - ctx, - "gh", "api", - fmt.Sprintf( - "repos/%s/pulls/%d/comments?"+ - "sort=created&direction=asc", // determinism: always patch the same comment id - repo, - prNumber, - ), - "--paginate", - ) - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("gh api list comments: %w (stderr: %s)", err, stderr.String()) - } - - var existingComments []existingReviewComment - respBytes := stdout.Bytes() - if err := json.Unmarshal(respBytes, &existingComments); err != nil { - return nil, fmt.Errorf("parse comments response: %s: %w", string(respBytes), err) - } - +// github-actions[bot] on the PR. Sorted ascending by creation time so that when multiple bot +// comments exist at the same file+line, the highest-ID (most recently created) one wins. +func listExistingBotComments(ctx context.Context, client *githubutil.Client, prNumber int) (map[commentKey]int64, error) { const githubActionsBotUsername = "github-actions[bot]" result := make(map[commentKey]int64) - for _, comment := range existingComments { - if comment.User.Login == githubActionsBotUsername { - result[commentKey{path: comment.Path, line: comment.Line}] = comment.ID + opts := &github.PullRequestListCommentsOptions{ + Sort: "created", + Direction: "asc", + ListOptions: github.ListOptions{PerPage: 100}, + } + for { + comments, resp, err := client.GitHub.PullRequests.ListComments( + ctx, + string(githubutil.GithubOwnerBufbuild), + string(githubutil.GithubRepoModules), + prNumber, + opts, + ) + if err != nil { + return nil, fmt.Errorf("list PR comments: %w", err) } + for _, comment := range comments { + if comment.GetUser().GetLogin() == githubActionsBotUsername { + key := commentKey{path: comment.GetPath(), line: comment.GetLine()} + result[key] = comment.GetID() + } + } + if resp.NextPage == 0 { + break + } + opts.Page = resp.NextPage } return result, nil } -func postSingleReviewComment(ctx context.Context, repoOwnerAndName string, prNumber uint, gitCommitID string, comment prReviewComment) error { - commentBody := fmt.Sprintf("_[Posted at %s]_\n\n%s", time.Now().Format(time.RFC3339), comment.body) - requestBody := map[string]any{ - "commit_id": gitCommitID, - "path": comment.filePath, - "line": comment.lineNumber, - "body": commentBody, - } - requestJSON, err := json.Marshal(requestBody) - if err != nil { - return fmt.Errorf("marshal request: %w", err) - } - - var stderr bytes.Buffer - cmd := exec.CommandContext( //nolint:gosec +func postSingleReviewComment(ctx context.Context, client *githubutil.Client, prNumber int, gitCommitID string, comment prReviewComment) error { + body := fmt.Sprintf("_[Posted at %s]_\n\n%s", time.Now().Format(time.RFC3339), comment.body) + _, _, err := client.GitHub.PullRequests.CreateComment( ctx, - "gh", "api", - fmt.Sprintf("repos/%s/pulls/%d/comments", repoOwnerAndName, prNumber), - "-X", "POST", - "--input", "-", + string(githubutil.GithubOwnerBufbuild), + string(githubutil.GithubRepoModules), + prNumber, + &github.PullRequestComment{ + CommitID: github.String(gitCommitID), + Path: github.String(comment.filePath), + Line: github.Int(comment.lineNumber), + Body: github.String(body), + }, ) - cmd.Stdin = bytes.NewReader(requestJSON) - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("gh api post comment: %w (stderr: %s)", err, stderr.String()) + if err != nil { + return fmt.Errorf("create PR comment: %w", err) } return nil } -func updateReviewComment(ctx context.Context, repoOwnerAndName string, prCommentID int64, body string) error { - commentBody := fmt.Sprintf("_[Updated at %s]_\n\n%s", time.Now().Format(time.RFC3339), body) - requestJSON, err := json.Marshal(map[string]any{"body": commentBody}) - if err != nil { - return fmt.Errorf("marshal request: %w", err) - } - - var stderr bytes.Buffer - cmd := exec.CommandContext( //nolint:gosec +func updateReviewComment(ctx context.Context, client *githubutil.Client, prCommentID int64, body string) error { + body = fmt.Sprintf("_[Updated at %s]_\n\n%s", time.Now().Format(time.RFC3339), body) + _, _, err := client.GitHub.PullRequests.EditComment( ctx, - "gh", "api", - fmt.Sprintf("repos/%s/pulls/comments/%d", repoOwnerAndName, prCommentID), - "-X", "PATCH", - "--input", "-", + string(githubutil.GithubOwnerBufbuild), + string(githubutil.GithubRepoModules), + prCommentID, + &github.PullRequestComment{ + Body: github.String(body), + }, ) - cmd.Stdin = bytes.NewReader(requestJSON) - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("gh api patch comment: %w (stderr: %s)", err, stderr.String()) + if err != nil { + return fmt.Errorf("edit PR comment: %w", err) } return nil } From bc23dbe06bf8be6337b9ced9bf55053556d0a9a6 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 16:40:18 -0500 Subject: [PATCH 30/37] Use appcmd --- cmd/commentprcasdiff/main.go | 52 ++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index b1b7ef17..735533e7 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -18,29 +18,56 @@ import ( "cmp" "context" "errors" - "flag" "fmt" "os" "strconv" "strings" + "buf.build/go/app/appcmd" + "buf.build/go/app/appext" "buf.build/go/standard/xslices" + "github.com/bufbuild/buf/private/pkg/slogapp" "github.com/bufbuild/modules/private/bufpkg/bufstate" + "github.com/spf13/pflag" ) +const rootCmdName = "commentprcasdiff" + func main() { - if err := run(context.Background()); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) + appcmd.Main(context.Background(), newCommand(rootCmdName)) +} + +func newCommand(name string) *appcmd.Command { + builder := appext.NewBuilder( + name, + appext.BuilderWithLoggerProvider(slogapp.LoggerProvider), + ) + flags := newFlags() + return &appcmd.Command{ + Use: name, + Short: "Post CAS diff comments on PRs when module digests change.", + BindFlags: flags.bind, + Run: builder.NewRunFunc( + func(ctx context.Context, _ appext.Container) error { + return run(ctx, flags) + }, + ), } } -func run(ctx context.Context) error { - var dryRun bool - flag.BoolVar(&dryRun, "dry-run", false, "print comments to stdout instead of posting to GitHub") - flag.Parse() +type flags struct { + dryRun bool +} + +func newFlags() *flags { + return &flags{} +} + +func (f *flags) bind(flagSet *pflag.FlagSet) { + flagSet.BoolVar(&f.dryRun, "dry-run", false, "print comments to stdout instead of posting to GitHub") +} - // Read environment variables +func run(ctx context.Context, flags *flags) error { baseRef := os.Getenv("BASE_REF") headRef := os.Getenv("HEAD_REF") prNumberString := os.Getenv("PR_NUMBER") @@ -52,7 +79,7 @@ func run(ctx context.Context) error { if headRef == "" { return errors.New("HEAD_REF environment variable is required") } - if !dryRun { + if !flags.dryRun { if os.Getenv("GITHUB_TOKEN") == "" { return errors.New("GITHUB_TOKEN environment variable is required when not a dry-run") } @@ -74,7 +101,6 @@ func run(ctx context.Context) error { headRef, ) - // Find changed module state directories moduleStatePaths, err := changedModuleStateFiles(ctx, baseRef, headRef) if err != nil { return fmt.Errorf("find changed modules: %w", err) @@ -103,7 +129,6 @@ func run(ctx context.Context) error { return fmt.Errorf("new state read writer: %w", err) } - // Collect all transitions from all modules var allTransitions []stateTransition for _, moduleStatePath := range moduleStatePathsSorted { fmt.Fprintf(os.Stdout, "Analyzing %s...\n", moduleStatePath) @@ -152,9 +177,8 @@ func run(ctx context.Context) error { } } - // Post or print comments for successful results if len(casResults) > 0 { - if dryRun { + if flags.dryRun { fmt.Fprintf(os.Stdout, "\n[dry-run] %d comment(s) would be posted:\n", len(casResults)) for _, result := range casResults { fmt.Fprintf(os.Stdout, "\n--- %s (line %d: %s -> %s) ---\n%s\n", From 89a8f9b767bda128bf77ffb2c36b386d132884d2 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 16:44:05 -0500 Subject: [PATCH 31/37] Remove some state syncs --- modules/sync/bufbuild/protovalidate/state.json | 18 +----------------- modules/sync/state.json | 4 ++-- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/modules/sync/bufbuild/protovalidate/state.json b/modules/sync/bufbuild/protovalidate/state.json index 1f957b65..708c7d95 100644 --- a/modules/sync/bufbuild/protovalidate/state.json +++ b/modules/sync/bufbuild/protovalidate/state.json @@ -267,22 +267,6 @@ { "name": "v0.14.1", "digest": "0ef32b59cbbf249d9068e009725befd8109b8d2d6541f15184c8622711bc71d532bb3746d43d50ac051810044b82992b756a048df334b770b38571b2bb04331c" - }, - { - "name": "v0.14.2", - "digest": "0ef32b59cbbf249d9068e009725befd8109b8d2d6541f15184c8622711bc71d532bb3746d43d50ac051810044b82992b756a048df334b770b38571b2bb04331c" - }, - { - "name": "v1.0.0", - "digest": "ddff976f6322abaecd49ff0b271b7f8dec485046bafc52fe31f3272b555fa2829328be24d71ececefca38c87319173a31f1e931984066531b6ca2016b3598146" - }, - { - "name": "v1.1.0", - "digest": "35b3d88f6b0fbf159d9eedfc2fbfa976490e6bca1d98914c1c71f29bfe2da6261fca56c057975b3e5c022b3234a0f2eea8e2d1b599a937c6c5d63d21201a9bc3" - }, - { - "name": "v1.1.1", - "digest": "2d64e8ff856e1bf78a1289c24e868bdbe1ee2196f32b10bfe6ad8a743c7db99ba4be1a75c21d9f865ffa28464e11d339365b0f7937c75f7b308b3d5993cefd25" } ] -} \ No newline at end of file +} diff --git a/modules/sync/state.json b/modules/sync/state.json index 9c0593f7..fbdf44d1 100644 --- a/modules/sync/state.json +++ b/modules/sync/state.json @@ -6,7 +6,7 @@ }, { "module_name": "bufbuild/protovalidate", - "latest_reference": "v1.1.1" + "latest_reference": "v0.14.1" }, { "module_name": "bufbuild/protovalidate-testing", @@ -81,4 +81,4 @@ "latest_reference": "v34.0" } ] -} \ No newline at end of file +} From 9d758d7874eb6b26f483d2cadf267ce42d63255c Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 16:44:31 -0500 Subject: [PATCH 32/37] Revert "Remove some state syncs" This reverts commit 89a8f9b767bda128bf77ffb2c36b386d132884d2. --- modules/sync/bufbuild/protovalidate/state.json | 18 +++++++++++++++++- modules/sync/state.json | 4 ++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/modules/sync/bufbuild/protovalidate/state.json b/modules/sync/bufbuild/protovalidate/state.json index 708c7d95..1f957b65 100644 --- a/modules/sync/bufbuild/protovalidate/state.json +++ b/modules/sync/bufbuild/protovalidate/state.json @@ -267,6 +267,22 @@ { "name": "v0.14.1", "digest": "0ef32b59cbbf249d9068e009725befd8109b8d2d6541f15184c8622711bc71d532bb3746d43d50ac051810044b82992b756a048df334b770b38571b2bb04331c" + }, + { + "name": "v0.14.2", + "digest": "0ef32b59cbbf249d9068e009725befd8109b8d2d6541f15184c8622711bc71d532bb3746d43d50ac051810044b82992b756a048df334b770b38571b2bb04331c" + }, + { + "name": "v1.0.0", + "digest": "ddff976f6322abaecd49ff0b271b7f8dec485046bafc52fe31f3272b555fa2829328be24d71ececefca38c87319173a31f1e931984066531b6ca2016b3598146" + }, + { + "name": "v1.1.0", + "digest": "35b3d88f6b0fbf159d9eedfc2fbfa976490e6bca1d98914c1c71f29bfe2da6261fca56c057975b3e5c022b3234a0f2eea8e2d1b599a937c6c5d63d21201a9bc3" + }, + { + "name": "v1.1.1", + "digest": "2d64e8ff856e1bf78a1289c24e868bdbe1ee2196f32b10bfe6ad8a743c7db99ba4be1a75c21d9f865ffa28464e11d339365b0f7937c75f7b308b3d5993cefd25" } ] -} +} \ No newline at end of file diff --git a/modules/sync/state.json b/modules/sync/state.json index fbdf44d1..9c0593f7 100644 --- a/modules/sync/state.json +++ b/modules/sync/state.json @@ -6,7 +6,7 @@ }, { "module_name": "bufbuild/protovalidate", - "latest_reference": "v0.14.1" + "latest_reference": "v1.1.1" }, { "module_name": "bufbuild/protovalidate-testing", @@ -81,4 +81,4 @@ "latest_reference": "v34.0" } ] -} +} \ No newline at end of file From 5e42e6fd267c1b0e771dfd38df6677d788035129 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 16:49:33 -0500 Subject: [PATCH 33/37] lint --- cmd/commentprcasdiff/comment_poster.go | 16 ++++++++-------- cmd/commentprcasdiff/main.go | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index 91a22b07..8a624d54 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -39,10 +39,10 @@ type commentKey struct { // postReviewComments posts review comments to specific lines in the PR diff. If a bot comment // already exists at the same file/line, it is updated instead of creating a duplicate. -func postReviewComments(ctx context.Context, prNumber uint, gitCommitID string, comments ...prReviewComment) error { +func postReviewComments(ctx context.Context, prNumber int, gitCommitID string, comments ...prReviewComment) error { client := githubutil.NewClient(ctx) - existingPRComments, err := listExistingBotComments(ctx, client, int(prNumber)) + existingPRComments, err := listExistingBotComments(ctx, client, prNumber) if err != nil { return fmt.Errorf("list existing bot comments: %w", err) } @@ -55,7 +55,7 @@ func postReviewComments(ctx context.Context, prNumber uint, gitCommitID string, errsPosting = append(errsPosting, fmt.Errorf("updating existing comment in %v: %w", key, err)) } } else { - if err := postSingleReviewComment(ctx, client, int(prNumber), gitCommitID, comment); err != nil { + if err := postSingleReviewComment(ctx, client, prNumber, gitCommitID, comment); err != nil { errsPosting = append(errsPosting, fmt.Errorf("posting new comment in %v: %w", key, err)) } } @@ -107,10 +107,10 @@ func postSingleReviewComment(ctx context.Context, client *githubutil.Client, prN string(githubutil.GithubRepoModules), prNumber, &github.PullRequestComment{ - CommitID: github.String(gitCommitID), - Path: github.String(comment.filePath), - Line: github.Int(comment.lineNumber), - Body: github.String(body), + CommitID: &gitCommitID, + Path: &comment.filePath, + Line: &comment.lineNumber, + Body: &body, }, ) if err != nil { @@ -127,7 +127,7 @@ func updateReviewComment(ctx context.Context, client *githubutil.Client, prComme string(githubutil.GithubRepoModules), prCommentID, &github.PullRequestComment{ - Body: github.String(body), + Body: &body, }, ) if err != nil { diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index 735533e7..15fccf44 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -71,7 +71,7 @@ func run(ctx context.Context, flags *flags) error { baseRef := os.Getenv("BASE_REF") headRef := os.Getenv("HEAD_REF") prNumberString := os.Getenv("PR_NUMBER") - var prNumber uint64 + var prNumber int if baseRef == "" { return errors.New("BASE_REF environment variable is required") @@ -87,7 +87,7 @@ func run(ctx context.Context, flags *flags) error { return errors.New("PR_NUMBER environment variable is required when not a dry-run") } var err error - prNumber, err = strconv.ParseUint(prNumberString, 10, 64) + prNumber, err = strconv.Atoi(prNumberString) if err != nil { return fmt.Errorf("parse PR_NUMBER: %w", err) } @@ -199,7 +199,7 @@ func run(ctx context.Context, flags *flags) error { body: result.output, } } - if err := postReviewComments(ctx, uint(prNumber), headRef, comments...); err != nil { + if err := postReviewComments(ctx, prNumber, headRef, comments...); err != nil { errsToReturn = append(errsToReturn, fmt.Errorf("post review comments: %w", err)) } } From 6d12ab5cb02e5bdf7bee63f40b3ed9cd9693df84 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 16:51:05 -0500 Subject: [PATCH 34/37] Run workflow in all PRs --- .github/workflows/auto-casdiff-comment.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/auto-casdiff-comment.yaml b/.github/workflows/auto-casdiff-comment.yaml index 96e83864..cf181e57 100644 --- a/.github/workflows/auto-casdiff-comment.yaml +++ b/.github/workflows/auto-casdiff-comment.yaml @@ -3,8 +3,9 @@ name: Auto-comment PR with casdiff on: pull_request: types: [opened, synchronize] - branches: - - main + # FIXME: filter to only PRs targetting main + # branches: + # - main paths: - modules/sync/*/*/state.json From ed1a13c64ec32dd741a29ae64540e39a5ef2c71f Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 17:06:20 -0500 Subject: [PATCH 35/37] Log comment URLs --- cmd/commentprcasdiff/comment_poster.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/commentprcasdiff/comment_poster.go b/cmd/commentprcasdiff/comment_poster.go index 8a624d54..3c0a16f4 100644 --- a/cmd/commentprcasdiff/comment_poster.go +++ b/cmd/commentprcasdiff/comment_poster.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "os" "time" "github.com/bufbuild/modules/internal/githubutil" @@ -101,7 +102,7 @@ func listExistingBotComments(ctx context.Context, client *githubutil.Client, prN func postSingleReviewComment(ctx context.Context, client *githubutil.Client, prNumber int, gitCommitID string, comment prReviewComment) error { body := fmt.Sprintf("_[Posted at %s]_\n\n%s", time.Now().Format(time.RFC3339), comment.body) - _, _, err := client.GitHub.PullRequests.CreateComment( + created, _, err := client.GitHub.PullRequests.CreateComment( ctx, string(githubutil.GithubOwnerBufbuild), string(githubutil.GithubRepoModules), @@ -116,12 +117,13 @@ func postSingleReviewComment(ctx context.Context, client *githubutil.Client, prN if err != nil { return fmt.Errorf("create PR comment: %w", err) } + fmt.Fprintf(os.Stdout, "Posted comment: %s\n", created.GetHTMLURL()) return nil } func updateReviewComment(ctx context.Context, client *githubutil.Client, prCommentID int64, body string) error { body = fmt.Sprintf("_[Updated at %s]_\n\n%s", time.Now().Format(time.RFC3339), body) - _, _, err := client.GitHub.PullRequests.EditComment( + updated, _, err := client.GitHub.PullRequests.EditComment( ctx, string(githubutil.GithubOwnerBufbuild), string(githubutil.GithubRepoModules), @@ -133,5 +135,6 @@ func updateReviewComment(ctx context.Context, client *githubutil.Client, prComme if err != nil { return fmt.Errorf("edit PR comment: %w", err) } + fmt.Fprintf(os.Stdout, "Updated comment: %s\n", updated.GetHTMLURL()) return nil } From 1ed486b42214db5b07aec6c28bc19f8b8c93d3d0 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 17:08:48 -0500 Subject: [PATCH 36/37] Revert "Run workflow in all PRs" This reverts commit 6d12ab5cb02e5bdf7bee63f40b3ed9cd9693df84. --- .github/workflows/auto-casdiff-comment.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/auto-casdiff-comment.yaml b/.github/workflows/auto-casdiff-comment.yaml index cf181e57..96e83864 100644 --- a/.github/workflows/auto-casdiff-comment.yaml +++ b/.github/workflows/auto-casdiff-comment.yaml @@ -3,9 +3,8 @@ name: Auto-comment PR with casdiff on: pull_request: types: [opened, synchronize] - # FIXME: filter to only PRs targetting main - # branches: - # - main + branches: + - main paths: - modules/sync/*/*/state.json From 9c72750dd1b83ae9bbf8e8b03847bb12f29f59ba Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 18 Mar 2026 17:54:38 -0500 Subject: [PATCH 37/37] Fix off-by-one refs indexes --- cmd/commentprcasdiff/state_analyzer.go | 82 +++++++++----- cmd/commentprcasdiff/state_analyzer_test.go | 116 ++++++++++++++++++++ 2 files changed, 168 insertions(+), 30 deletions(-) diff --git a/cmd/commentprcasdiff/state_analyzer.go b/cmd/commentprcasdiff/state_analyzer.go index 0f3d1ca0..b9ee220e 100644 --- a/cmd/commentprcasdiff/state_analyzer.go +++ b/cmd/commentprcasdiff/state_analyzer.go @@ -25,6 +25,7 @@ import ( "strings" "github.com/bufbuild/modules/private/bufpkg/bufstate" + statev1alpha1 "github.com/bufbuild/modules/private/gen/modules/state/v1alpha1" ) // stateTransition represents a digest change in a module's state.json file. @@ -72,41 +73,26 @@ func getStateFileTransitions( // update/remove existing refs in base. baseRefs := baseState.GetReferences() headRefs := headState.GetReferences() - baseRefsCount := len(baseRefs) - headRefsCount := len(headRefs) - if headRefsCount <= baseRefsCount { - // No new references added + current, appendedRefs := resolveAppendedRefs(baseRefs, headRefs) + if current == nil { return nil, nil } - appendedRefs := headRefs[baseRefsCount:] - - // Get the baseline digest (last reference in base state) - var ( - currentDigest string - currentRef string - ) - if baseRefsCount > 0 { - lastBaseRef := baseRefs[baseRefsCount-1] - currentDigest = lastBaseRef.GetDigest() - currentRef = lastBaseRef.GetName() - } else if len(appendedRefs) > 0 { - // If base state is empty, use first appended ref as baseline - currentDigest = appendedRefs[0].GetDigest() - currentRef = appendedRefs[0].GetName() - appendedRefs = appendedRefs[1:] // Skip first as it's now the baseline - } // Get line number mapping for the appended references - lineNumbers, err := getLineNumbersForAppendedRefs(ctx, filePath, baseRef, headRef, baseRefsCount, headRefsCount) + lineNumbers, err := getLineNumbersForAppendedRefs(ctx, filePath, baseRef, headRef, len(baseRefs), len(headRefs)) if err != nil { return nil, fmt.Errorf("get line numbers: %w", err) } // Detect digest transitions - var transitions []stateTransition - modulePath := filepath.Dir(filePath) - for i, ref := range appendedRefs { - if ref.GetDigest() != currentDigest { + var ( + modulePath = filepath.Dir(filePath) + currentRef = current.GetName() + currentDigest = current.GetDigest() + transitions []stateTransition + ) + for i, appendedRef := range appendedRefs { + if appendedRef.GetDigest() != currentDigest { // Digest changed! Record transition var lineNumber int if i < len(lineNumbers) { @@ -116,19 +102,55 @@ func getStateFileTransitions( modulePath: modulePath, filePath: filePath, fromRef: currentRef, - toRef: ref.GetName(), + toRef: appendedRef.GetName(), fromDigest: currentDigest, - toDigest: ref.GetDigest(), + toDigest: appendedRef.GetDigest(), lineNumber: lineNumber, }) - currentDigest = ref.GetDigest() + currentDigest = appendedRef.GetDigest() } - currentRef = ref.GetName() + currentRef = appendedRef.GetName() } return transitions, nil } +// resolveAppendedRefs identifies the refs from headRefs that are new, considering new by +// index-behavior, not its content. Meaning if base has 3 refs and head has 5, we assume the first 3 +// in head are the same 3 in base, and the latest 2 are "appended". That is the use case for the +// fetch-modules PR, and that is for now the only supported behavior. +// +// Returns (nil, nil) when the head refs is empty. The current ref is the latest in the base refs. +// If base is empty, the first ref in head is returned as the current, and all the rest are returned +// as appended. +func resolveAppendedRefs( + baseRefs []*statev1alpha1.ModuleReference, + headRefs []*statev1alpha1.ModuleReference, +) (*statev1alpha1.ModuleReference, []*statev1alpha1.ModuleReference) { + if len(baseRefs) == 0 && len(headRefs) == 0 { + // both empty + // - current: none + // - appended: none + return nil, nil + } + if len(baseRefs) == 0 { + // base empty + // - current: first in head + // - appended: the rest from head (if any) + return headRefs[0], headRefs[1:] + } + if len(headRefs) <= len(baseRefs) { + // head has the same amount or less refs than base + // - current: last in base + // - appended: none + return baseRefs[len(baseRefs)-1], nil + } + // head has more refs than base (expected scenario most of the times) + // - current: last in base + // - appended: the index-base appended in head + return baseRefs[len(baseRefs)-1], headRefs[len(baseRefs):] +} + // readFileAtRef reads a file's content at a specific git ref using git show. func readFileAtRef(ctx context.Context, filePath string, ref string) ([]byte, error) { cmd := exec.CommandContext(ctx, "git", "show", fmt.Sprintf("%s:%s", ref, filePath)) //nolint:gosec diff --git a/cmd/commentprcasdiff/state_analyzer_test.go b/cmd/commentprcasdiff/state_analyzer_test.go index 252d6e35..f9191823 100644 --- a/cmd/commentprcasdiff/state_analyzer_test.go +++ b/cmd/commentprcasdiff/state_analyzer_test.go @@ -17,10 +17,126 @@ package main import ( "testing" + statev1alpha1 "github.com/bufbuild/modules/private/gen/modules/state/v1alpha1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestResolveAppendedRefs(t *testing.T) { + t.Parallel() + ref := func(name, digest string) *statev1alpha1.ModuleReference { + return statev1alpha1.ModuleReference_builder{Name: name, Digest: digest}.Build() + } + type testCase struct { + name string + baseRefs []*statev1alpha1.ModuleReference + headRefs []*statev1alpha1.ModuleReference + wantCurrent *statev1alpha1.ModuleReference + wantAppended []*statev1alpha1.ModuleReference + } + testCases := []testCase{ + { + // base=0, head=0 → nothing to do + name: "both_empty", + baseRefs: nil, + headRefs: nil, + wantCurrent: nil, + wantAppended: nil, + }, + { + // base=0, head=1 → head[0] is baseline, nothing appended + name: "base_empty_single_ref_in_head", + baseRefs: nil, + headRefs: []*statev1alpha1.ModuleReference{ref("v1.0.0", "d1")}, + wantCurrent: ref("v1.0.0", "d1"), + wantAppended: []*statev1alpha1.ModuleReference{}, + }, + { + // base=0, head=3 → head[0] is baseline, head[1:] are appended + name: "base_empty_multiple_refs", + baseRefs: nil, + headRefs: []*statev1alpha1.ModuleReference{ + ref("v1.0.0", "d1"), + ref("v2.0.0", "d2"), + ref("v3.0.0", "d2"), + }, + wantCurrent: ref("v1.0.0", "d1"), + wantAppended: []*statev1alpha1.ModuleReference{ + ref("v2.0.0", "d2"), + ref("v3.0.0", "d2"), + }, + }, + { + // base=2, head=4 → base[latest] is baseline, head[2:] are appended + name: "base_non_empty_some_appends", + baseRefs: []*statev1alpha1.ModuleReference{ + ref("v1.0.0", "d1"), + ref("v1.1.0", "d1"), + }, + headRefs: []*statev1alpha1.ModuleReference{ + ref("v1.0.0", "d1"), + ref("v1.1.0", "d1"), + ref("v2.0.0", "d2"), + ref("v2.1.0", "d2"), + }, + wantCurrent: ref("v1.1.0", "d1"), + wantAppended: []*statev1alpha1.ModuleReference{ + ref("v2.0.0", "d2"), + ref("v2.1.0", "d2"), + }, + }, + { + // base=1, head=1 → base[latest] is baseline, no appends + name: "head_count_equals_base_count_not_supported", + baseRefs: []*statev1alpha1.ModuleReference{ref("v1.0.0", "d1")}, + headRefs: []*statev1alpha1.ModuleReference{ref("v1.0.0", "d1")}, + wantCurrent: ref("v1.0.0", "d1"), + wantAppended: nil, + }, + { + // base=2, head=1 → base[latest] is baseline, no appends + name: "head_shorter_than_base_not_supported", + baseRefs: []*statev1alpha1.ModuleReference{ + ref("v1.0.0", "d1"), + ref("v2.0.0", "d2"), + }, + headRefs: []*statev1alpha1.ModuleReference{ref("v1.0.0", "d1")}, + wantCurrent: ref("v2.0.0", "d2"), + wantAppended: nil, + }, + { + // base=3, head=5, but the digests don't match. The function only looks at counts, not + // content: base[latest] is baseline, head[2:] are appended. + name: "existing_ref_modified_not_supported", + baseRefs: []*statev1alpha1.ModuleReference{ + ref("v1.0.0", "d1"), + ref("v2.0.0", "d2"), + ref("v3.0.0", "d3"), + }, + headRefs: []*statev1alpha1.ModuleReference{ + ref("v1.0.0", "d1-modified"), + ref("v2.0.0", "d2-modified"), + ref("v3.0.0", "d3-modified"), + ref("v4.0.0", "d4"), + ref("v5.0.0", "d5"), + }, + wantCurrent: ref("v3.0.0", "d3"), // the one from base, not from head + wantAppended: []*statev1alpha1.ModuleReference{ + ref("v4.0.0", "d4"), + ref("v5.0.0", "d5"), + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + gotCurrent, gotAppended := resolveAppendedRefs(tc.baseRefs, tc.headRefs) + assert.Equal(t, tc.wantCurrent, gotCurrent) + assert.Equal(t, tc.wantAppended, gotAppended) + }) + } +} + func TestParseLineNumbersFromDiff(t *testing.T) { t.Parallel() type testCase struct {