diff --git a/go/cmd/gitter/errors.go b/go/cmd/gitter/errors.go index 92459ad1601..f1a3dd28782 100644 --- a/go/cmd/gitter/errors.go +++ b/go/cmd/gitter/errors.go @@ -156,11 +156,20 @@ func isRefNotFoundError(err error) bool { return errContainsAny(err, "not found or invalid", "failed to resolve target ref", + "failed to resolve target branch", "failed to run git rev-parse", "ref cannot be empty", ) } +// isCommitNotAncestorError returns true if last_scan_commit is not found in the repository or is not an ancestor of HEAD. +func isCommitNotAncestorError(err error) bool { + return errContainsAny(err, + "not an ancestor", + "failed to resolve last_scan_commit", + ) +} + // isFileNotFoundError returns true if a file path does not exist at the requested commit. func isFileNotFoundError(err error) bool { return errContainsAny(err, diff --git a/go/cmd/gitter/git.go b/go/cmd/gitter/git.go index b76f68f5ff1..62f8b3c8c54 100644 --- a/go/cmd/gitter/git.go +++ b/go/cmd/gitter/git.go @@ -42,8 +42,8 @@ func prepareCmd(ctx context.Context, dir string, env []string, name string, args } // runCmd executes a command with context cancellation handled by sending SIGINT. -// It logs cancellation errors separately as requested. -func runCmd(ctx context.Context, dir string, env []string, name string, args ...string) error { +// It logs cancellation errors separately as requested +func runCmd(ctx context.Context, dir string, env []string, name string, args ...string) ([]byte, error) { cmd := prepareCmd(ctx, dir, env, name, args...) out, err := cmd.CombinedOutput() @@ -51,10 +51,10 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... if ctx.Err() != nil { // Log separately if cancelled logger.DebugContext(ctx, "Command cancelled", slog.String("cmd", name), slog.Any("err", ctx.Err())) - return fmt.Errorf("command %s cancelled: %w", name, ctx.Err()) + return out, fmt.Errorf("command %s cancelled: %w", name, ctx.Err()) } - return fmt.Errorf("command %s failed: %w, output: %s", name, err, out) + return out, fmt.Errorf("command %s failed: %w, output: %s", name, err, out) } logger.DebugContext(ctx, "Git command executed", @@ -63,12 +63,13 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... slog.String("output", string(out)), ) - return nil + return out, nil } // cloneRepo clones a git repository into repoPath. func cloneRepo(ctx context.Context, repoURL string, repoPath string) error { - return runCmd(ctx, "", []string{"GIT_TERMINAL_PROMPT=0"}, "git", "clone", "--", repoURL, repoPath) + _, err := runCmd(ctx, "", []string{"GIT_TERMINAL_PROMPT=0"}, "git", "clone", "--", repoURL, repoPath) + return err } // Attempt to recover from git fetch errors @@ -82,7 +83,7 @@ func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { // We can try removing stale remote-tracking branches and retry if isRefConflictError(err) { logger.WarnContext(ctx, "Ref conflict detected, running git remote prune origin") - if err := runCmd(ctx, repoPath, nil, "git", "remote", "prune", "origin"); err != nil { + if _, err := runCmd(ctx, repoPath, nil, "git", "remote", "prune", "origin"); err != nil { logger.WarnContext(ctx, "Failed to prune origin", slog.Any("err", err)) return false } @@ -101,13 +102,13 @@ func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { // fetchRepo fetches remote origin and updates origin/HEAD to the remote's default branch func fetchRepo(ctx context.Context, repoPath string) error { - err := runCmd(ctx, repoPath, nil, "git", "fetch", "origin") + _, err := runCmd(ctx, repoPath, nil, "git", "fetch", "origin") if err != nil { return fmt.Errorf("git fetch failed: %w", err) } // Make sure origin/HEAD points to the latest default branch from remotes - err = runCmd(ctx, repoPath, nil, "git", "remote", "set-head", "origin", "--auto") + _, err = runCmd(ctx, repoPath, nil, "git", "remote", "set-head", "origin", "--auto") if err != nil { logger.WarnContext(ctx, "git remote set-head failed", slog.Any("err", err)) } @@ -297,14 +298,14 @@ func ArchiveRepo(ctx context.Context, repoURL string) ([]byte, error) { startArchive := time.Now() // Reset working tree to origin/HEAD before creating archive - if err := runCmd(ctx, repoPath, nil, "git", "reset", "--hard", "origin/HEAD"); err != nil { + if _, err := runCmd(ctx, repoPath, nil, "git", "reset", "--hard", "origin/HEAD"); err != nil { return nil, fmt.Errorf("git reset failed: %w", err) } // Archive // tar --zstd -cf -C "/" . // using -C to archive the relative path so it unzips nicely - err := runCmd(ctx, "", nil, "tar", "--zstd", "-cf", archivePath, "-C", filepath.Join(gitStorePath, repoDirName), ".") + _, err := runCmd(ctx, "", nil, "tar", "--zstd", "-cf", archivePath, "-C", filepath.Join(gitStorePath, repoDirName), ".") if err != nil { return nil, fmt.Errorf("tar zstd failed: %w", err) } diff --git a/go/cmd/gitter/git_test.go b/go/cmd/gitter/git_test.go index 2a7ef322012..5884a0bad45 100644 --- a/go/cmd/gitter/git_test.go +++ b/go/cmd/gitter/git_test.go @@ -47,14 +47,14 @@ func TestRefreshRepo_DontRecloneOnRemoteError(t *testing.T) { if err := os.MkdirAll(repoPath, 0755); err != nil { t.Fatalf("failed to create repo dir: %v", err) } - if err := runCmd(ctx, repoPath, nil, "git", "init"); err != nil { + if _, err := runCmd(ctx, repoPath, nil, "git", "init"); err != nil { t.Fatalf("git init failed: %v", err) } // Configure dummy git user - _ = runCmd(ctx, repoPath, nil, "git", "config", "user.name", "Test") - _ = runCmd(ctx, repoPath, nil, "git", "config", "user.email", "test@example.com") + _, _ = runCmd(ctx, repoPath, nil, "git", "config", "user.name", "Test") + _, _ = runCmd(ctx, repoPath, nil, "git", "config", "user.email", "test@example.com") // Add remote origin pointing to an unreachable port (connection refused) - if err := runCmd(ctx, repoPath, nil, "git", "remote", "add", "origin", "https://127.0.0.1:59999/repo.git"); err != nil { + if _, err := runCmd(ctx, repoPath, nil, "git", "remote", "add", "origin", "https://127.0.0.1:59999/repo.git"); err != nil { t.Fatalf("git remote add failed: %v", err) } @@ -63,10 +63,10 @@ func TestRefreshRepo_DontRecloneOnRemoteError(t *testing.T) { if err := os.WriteFile(dummyFile, []byte("hello"), 0600); err != nil { t.Fatalf("write file failed: %v", err) } - if err := runCmd(ctx, repoPath, nil, "git", "add", "dummy.txt"); err != nil { + if _, err := runCmd(ctx, repoPath, nil, "git", "add", "dummy.txt"); err != nil { t.Fatalf("git add failed: %v", err) } - if err := runCmd(ctx, repoPath, nil, "git", "commit", "-m", "initial commit"); err != nil { + if _, err := runCmd(ctx, repoPath, nil, "git", "commit", "-m", "initial commit"); err != nil { t.Fatalf("git commit failed: %v", err) } diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index 9a65d75eddc..3779c247179 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -33,6 +33,7 @@ import ( "golang.org/x/sync/singleflight" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" pb "github.com/google/osv.dev/go/internal/gitter/pb/repository" ) @@ -56,6 +57,7 @@ var endpointHandlers = map[string]http.HandlerFunc{ "POST /affected-commits": affectedCommitsHandler, "POST /file-diffs": fileDiffsHandler, "POST /file-content": fileContentHandler, + "POST /commit-diffs": commitDiffsHandler, } var ( @@ -965,3 +967,110 @@ func fileContentHandler(w http.ResponseWriter, req *http.Request) { http.Error(w, fmt.Sprintf("Error writing file content response: %v", err), statusCode) } } + +func commitDiffsHandler(w http.ResponseWriter, req *http.Request) { + start := time.Now() + statusCode := http.StatusOK + ctx := req.Context() + defer func() { logRequestCompletion(ctx, "/commit-diffs", start, statusCode) }() + + body := &pb.CommitDiffsRequest{} + if err := unmarshalRequest(req, body); err != nil { + statusCode = http.StatusBadRequest + http.Error(w, fmt.Sprintf("Error unmarshaling request: %v", err), statusCode) + + return + } + + repoURL, err := prepareURL(req, body.GetUrl()) + if err != nil { + statusCode = http.StatusBadRequest + http.Error(w, err.Error(), statusCode) + + return + } + + lastScanCommit := strings.TrimSpace(body.GetLastScanCommit()) + branch := strings.TrimSpace(body.GetBranch()) + var lastScanTime time.Time + if body.GetLastScanTime() != nil { + lastScanTime = body.GetLastScanTime().AsTime() + } + + // Require at least one boundary (last_scan_commit or last_scan_time). + if lastScanCommit == "" && lastScanTime.IsZero() { + statusCode = http.StatusBadRequest + http.Error(w, "missing last_scan_commit and last_scan_time (must provide at least one)", statusCode) + + return + } + + ctx = context.WithValue(ctx, urlKey, repoURL) + logger.DebugContext(ctx, "Received request: /commit-diffs", + slog.String("last_scan_commit", lastScanCommit), + slog.String("branch", branch), + slog.Time("last_scan_time", lastScanTime), + slog.Bool("newest_first", body.GetNewestFirst()), + ) + + // Always force update to get latest commits from the remote + repo, err := SyncRepoOnDisk(ctx, repoURL, FetchOptions{ForceUpdate: true, SkipReqConcurrencySemaphore: false}) + if err != nil { + statusCode = errorToHTTPStatusCode(err) + cacheInvalidRepo(repoURL, statusCode) + http.Error(w, fmt.Sprintf("Error getting repo: %v", err), statusCode) + + return + } + + resolvedBranch, headCommit, commits, err := repo.ListCommits(ctx, branch, lastScanCommit, lastScanTime, body.GetNewestFirst(), body.GetIncludePaths(), body.GetExcludePaths()) + if err != nil { + // Distinguish missing refs - 404 Not Found (e.g. the commit hash is not a valid ancestor of HEAD - likely from git amend) + // From generic issues - 500 + if isRefNotFoundError(err) || isCommitNotAncestorError(err) { + statusCode = http.StatusNotFound + http.Error(w, fmt.Sprintf("Commit or branch not found: %v", err), statusCode) + + return + } + logger.ErrorContext(ctx, "Error listing commits", slog.Any("error", err)) + statusCode = http.StatusInternalServerError + http.Error(w, fmt.Sprintf("Error listing commits: %v", err), statusCode) + + return + } + + pbCommits := make([]*pb.CommitDiff, 0, len(commits)) + for _, c := range commits { + filesChanged := make([]*pb.FileChange, 0, len(c.FilesChanged)) + for _, f := range c.FilesChanged { + filesChanged = append(filesChanged, &pb.FileChange{ + FromPath: f.From, + ToPath: f.To, + }) + } + pbCommits = append(pbCommits, &pb.CommitDiff{ + Commit: c.Commit, + Timestamp: timestamppb.New(c.Timestamp), + Message: c.Message, + Patch: c.Patch, + FilesChanged: filesChanged, + PatchTruncated: c.PatchTruncated, + }) + } + + resp := &pb.CommitDiffsResponse{ + Url: body.GetUrl(), + Branch: resolvedBranch, + HeadCommit: headCommit, + //nolint:gosec // G115: len(commits) should safely fit in int32 (max is 2.14 billion) + NumCommits: int32(len(commits)), + Commits: pbCommits, + } + + if err := writeResponse(w, req, resp); err != nil { + logger.ErrorContext(ctx, "Error writing commit diffs response", slog.Any("error", err)) + statusCode = http.StatusInternalServerError + http.Error(w, fmt.Sprintf("Error writing commit diffs response: %v", err), statusCode) + } +} diff --git a/go/cmd/gitter/gitter_test.go b/go/cmd/gitter/gitter_test.go index 55f68f26e55..813ed964ffd 100644 --- a/go/cmd/gitter/gitter_test.go +++ b/go/cmd/gitter/gitter_test.go @@ -13,6 +13,7 @@ import ( "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/testing/protocmp" + "google.golang.org/protobuf/types/known/timestamppb" ) func TestGetRepoDirName(t *testing.T) { @@ -770,3 +771,182 @@ func TestFileContentHandler(t *testing.T) { }) } } + +func TestCommitDiffsHandler(t *testing.T) { + setupTest(t) + + tests := []struct { + name string + reqBody *pb.CommitDiffsRequest + rawBody []byte + contentType string + expectedCode int + verifyResp func(t *testing.T, resp *pb.CommitDiffsResponse) + }{ + { + name: "Missing repo URL", + reqBody: &pb.CommitDiffsRequest{ + LastScanCommit: "1234567890123456789012345678901234567890", + }, + contentType: "application/json", + expectedCode: http.StatusBadRequest, + }, + { + name: "Missing both last_scan_commit and last_scan_time", + reqBody: &pb.CommitDiffsRequest{ + Url: "https://github.com/oliverchang/osv-test.git", + }, + contentType: "application/json", + expectedCode: http.StatusBadRequest, + }, + { + name: "Forbidden or non-existent repo", + reqBody: &pb.CommitDiffsRequest{ + Url: "https://github.com/google/this-repo-does-not-exist-12345.git", + LastScanCommit: "ff8cc32ba60ad9cbb3b23f0a82aad96ebe9ff76b", + }, + contentType: "application/json", + expectedCode: http.StatusForbidden, + }, + { + name: "Query commits with JSON", + reqBody: &pb.CommitDiffsRequest{ + Url: "https://github.com/oliverchang/osv-test.git", + LastScanCommit: "b1c95a196f22d06fcf80df8c6691cd113d8fefff", + }, + contentType: "application/json", + expectedCode: http.StatusOK, + verifyResp: func(t *testing.T, resp *pb.CommitDiffsResponse) { + t.Helper() + if resp.GetHeadCommit() != "b9b3fd4732695b83c3068b7b6a14bb372ec31f98" { + t.Errorf("unexpected head commit: %s", resp.GetHeadCommit()) + } + if resp.GetNumCommits() != 2 || len(resp.GetCommits()) != 2 { + t.Fatalf("expected 2 commits, got %d", resp.GetNumCommits()) + } + // Default order is oldest first, so the latest commit (HEAD) is the second commit + c := resp.GetCommits()[1] + if c.GetCommit() != "b9b3fd4732695b83c3068b7b6a14bb372ec31f98" { + t.Errorf("unexpected commit sha: %s", c.GetCommit()) + } + if c.GetPatch() == "" { + t.Errorf("expected non-empty patch") + } + if len(c.GetFilesChanged()) == 0 { + t.Errorf("expected non-empty files changed") + } + }, + }, + { + name: "Query commits with Protobuf wire format", + reqBody: &pb.CommitDiffsRequest{ + Url: "https://github.com/oliverchang/osv-test.git", + LastScanCommit: "b1c95a196f22d06fcf80df8c6691cd113d8fefff", + NewestFirst: true, + }, + contentType: "application/x-protobuf", + expectedCode: http.StatusOK, + verifyResp: func(t *testing.T, resp *pb.CommitDiffsResponse) { + t.Helper() + if resp.GetHeadCommit() != "b9b3fd4732695b83c3068b7b6a14bb372ec31f98" { + t.Errorf("unexpected head commit: %s", resp.GetHeadCommit()) + } + if resp.GetNumCommits() != 2 || len(resp.GetCommits()) != 2 { + t.Fatalf("expected 2 commits, got %d", resp.GetNumCommits()) + } + // Newest first, so first commit is HEAD + c := resp.GetCommits()[0] + if c.GetCommit() != "b9b3fd4732695b83c3068b7b6a14bb372ec31f98" { + t.Errorf("unexpected commit sha: %s", c.GetCommit()) + } + }, + }, + { + name: "Query commits using last_scan_time only", + reqBody: &pb.CommitDiffsRequest{ + Url: "https://github.com/oliverchang/osv-test.git", + LastScanTime: timestamppb.New(time.Unix(1, 0)), + }, + contentType: "application/json", + expectedCode: http.StatusOK, + verifyResp: func(t *testing.T, resp *pb.CommitDiffsResponse) { + t.Helper() + if resp.GetNumCommits() == 0 { + t.Errorf("expected non-empty commits") + } + }, + }, + { + name: "Non-existent branch name", + reqBody: &pb.CommitDiffsRequest{ + Url: "https://github.com/oliverchang/osv-test.git", + Branch: "non-existent-branch-12345", + LastScanCommit: "b1c95a196f22d06fcf80df8c6691cd113d8fefff", + }, + contentType: "application/json", + expectedCode: http.StatusNotFound, + }, + { + name: "Invalid last_scan_commit without last_scan_time", + reqBody: &pb.CommitDiffsRequest{ + Url: "https://github.com/oliverchang/osv-test.git", + LastScanCommit: "invalidhash123456", + }, + contentType: "application/json", + expectedCode: http.StatusNotFound, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var bodyBytes []byte + if tt.rawBody != nil { + bodyBytes = tt.rawBody + } else if tt.reqBody != nil { + var err error + if tt.contentType == "application/json" { + bodyBytes, err = protojson.Marshal(tt.reqBody) + } else { + bodyBytes, err = proto.Marshal(tt.reqBody) + } + if err != nil { + t.Fatalf("failed to marshal request: %v", err) + } + } + + req, err := http.NewRequest(http.MethodPost, "/commit-diffs", bytes.NewReader(bodyBytes)) + if err != nil { + t.Fatal(err) + } + if tt.contentType != "" { + req.Header.Set("Content-Type", tt.contentType) + } + + rr := httptest.NewRecorder() + commitDiffsHandler(rr, req) + + // Non-existent repos on github may return 403 or 502 depending on network latency/timeouts + if tt.name == "Forbidden or non-existent repo" { + if rr.Code != http.StatusForbidden && rr.Code != http.StatusBadGateway { + t.Errorf("commitDiffsHandler returned wrong status code: got %v want 403 or 502", rr.Code) + } + } else if status := rr.Code; status != tt.expectedCode { + t.Errorf("commitDiffsHandler returned wrong status code: got %v want %v", status, tt.expectedCode) + } + + if tt.verifyResp != nil && rr.Code == http.StatusOK { + resp := &pb.CommitDiffsResponse{} + var err error + if tt.contentType == "application/json" { + err = protojson.Unmarshal(rr.Body.Bytes(), resp) + } else { + err = proto.Unmarshal(rr.Body.Bytes(), resp) + } + if err != nil { + t.Fatalf("failed to unmarshal response: %v", err) + } + tt.verifyResp(t, resp) + } + }) + } +} diff --git a/go/cmd/gitter/repository.go b/go/cmd/gitter/repository.go index a76fec37827..c63d0973a4d 100644 --- a/go/cmd/gitter/repository.go +++ b/go/cmd/gitter/repository.go @@ -55,9 +55,27 @@ type Repository struct { rootCommits []int } -// %H commit hash; %P parent hashes; %D:refs (tab delimited) -// We use \x09 (tab) as delimiter because it is disallowed in git refs and won't appear in hashes -const gitLogFormat = "%H%x09%P%x09%D" +const ( + // gitLogGraphFormat formats commits for building the commit graph and patch IDs: + // %H commit hash; %P parent hashes; %D:refs (tab delimited) + // We use \x09 (tab) as delimiter because it is disallowed in git refs and won't appear in hashes + gitLogGraphFormat = "%H%x09%P%x09%D" + + commitMetadataTag = "---COMMIT-METADATA---" + commitDiffTag = "---COMMIT-DIFF---" + + // gitLogCommitDiffsFormat formats commit records for ListCommits: + // Format goes like this: + // %x1e (record separator) ---COMMIT-METADATA--- + // Metadata: %H (hash); %ct (commit timestamp); %B (raw commit message body) + // %x00 (NUL byte) ---COMMIT-DIFF--- + // The file diffs output + gitLogCommitDiffsFormat = "%x1e" + commitMetadataTag + "%n%H%n%ct%n%B%x00" + commitDiffTag + "%n" + + // Delimiters for metadata/diff extraction in ListCommits. + commitMetadataMarker = "\x1e" + commitMetadataTag + "\n" + commitDiffMarker = "\x00" + commitDiffTag + "\n" +) // Number of workers for patch ID calculation var workers = 16 @@ -188,7 +206,7 @@ func (r *Repository) buildCommitGraph(ctx context.Context, cache *pb.RepositoryC // --all: all branches // --full-history + --sparse: full-history alone still prunes TREESAME commit so we combine that with --sparse to actually get the full history of a repository // Redirecting to a file is faster than git binary's own --output flag or streaming into memory. - cmd := prepareCmd(ctx, r.repoPath, nil, "git", "log", "--all", "--full-history", "--sparse", "--format="+gitLogFormat) + cmd := prepareCmd(ctx, r.repoPath, nil, "git", "log", "--all", "--full-history", "--sparse", "--format="+gitLogGraphFormat) cmd.Stdout = tmpFile var stderr bytes.Buffer cmd.Stderr = &stderr @@ -1012,15 +1030,28 @@ type FileChange struct { To string } +// DefaultMaxCommitPatchBytes is the default maximum size (5 MB) allowed for an individual commit's unified patch diff. +// If a commit's patch exceeds this size, the patch is truncated to prevent excessive memory consumption. +const DefaultMaxCommitPatchBytes = 5 * 1024 * 1024 + +// CommitDiff represents a commit and its changes on a repository. +type CommitDiff struct { + Commit string + Timestamp time.Time + Message string + Patch string + FilesChanged []*FileChange + PatchTruncated bool +} + // resolveCommit resolves a branch or (abbreviated) commit SHA to its raw 20-byte SHA-1. func (r *Repository) resolveCommit(ctx context.Context, ref string) (string, error) { if strings.TrimSpace(ref) == "" { return "", errors.New("ref cannot be empty") } - cmd := prepareCmd(ctx, r.repoPath, nil, "git", "rev-parse", "--verify", "--quiet", ref+"^{commit}") - out, err := cmd.CombinedOutput() + out, err := runCmd(ctx, r.repoPath, nil, "git", "rev-parse", "--verify", "--quiet", ref+"^{commit}") if err != nil { - return "", fmt.Errorf("failed to run git rev-parse on %q: %w, output: %s", ref, err, out) + return "", fmt.Errorf("failed to run git rev-parse on %q: %w", ref, err) } return strings.TrimSpace(string(out)), nil @@ -1195,3 +1226,260 @@ func (r *Repository) GetFileContent(ctx context.Context, ref, path string) ([]by return out, nil } + +// ListCommits returns commits on targetBranch since lastScanCommit (or the lastScanTime timestamp). +// Optional includePaths and excludePaths to apply Git pathspec filtering. +func (r *Repository) ListCommits(ctx context.Context, targetBranch, lastScanCommit string, lastScanTime time.Time, newestFirst bool, includePaths, excludePaths []string) (string, string, []*CommitDiff, error) { + repoLock := GetRepoLock(r.URL) + repoLock.RLock() + defer repoLock.RUnlock() + + logger.DebugContext(ctx, "Starting commits listing", + slog.String("target_branch", targetBranch), + slog.String("last_scan_commit", lastScanCommit), + slog.Time("last_scan_time", lastScanTime), + slog.Bool("newest_first", newestFirst), + slog.Any("include_paths", includePaths), + slog.Any("exclude_paths", excludePaths), + ) + start := time.Now() + + // Step 1: Resolve the target branch name and HEAD commit SHA. + var ref string + var resolvedBranch string + if targetBranch == "" { + // If targetBranch is empty, default to origin/HEAD, but still need to find the actual branch name + ref = "origin/HEAD" + if out, err := runCmd(ctx, r.repoPath, nil, "git", "symbolic-ref", "--short", "refs/remotes/origin/HEAD"); err == nil { + resolvedBranch = strings.TrimPrefix(strings.TrimSpace(string(out)), "origin/") + } + } else { + resolvedBranch = targetBranch + if strings.HasPrefix(targetBranch, "origin/") { + ref = targetBranch + } else { + ref = "origin/" + targetBranch + } + } + + toCommit, err := r.resolveCommit(ctx, ref) + if err != nil { + // Fallback for local test fixtures without origin/ + toCommit, err = r.resolveCommit(ctx, targetBranch) + if err != nil { + return "", "", nil, fmt.Errorf("failed to resolve target branch %q: %w", targetBranch, err) + } + } + + // Step 2: Validate lastScanCommit + var fromCommit string + if lastScanCommit != "" { + if lastScanCommit == toCommit || strings.HasPrefix(toCommit, lastScanCommit) { + // No new commits exist, return early. + return resolvedBranch, toCommit, []*CommitDiff{}, nil + } + // Validates last scan commit is an ancestor of the target HEAD commit + _, err := runCmd(ctx, r.repoPath, nil, "git", "merge-base", "--is-ancestor", lastScanCommit, toCommit) + if err == nil { + fromCommit = lastScanCommit + } else { + // Fall back to last scan time + if lastScanTime.IsZero() { + return "", "", nil, fmt.Errorf("last_scan_commit %s is not an ancestor of HEAD commit %s and last_scan_time is not provided", lastScanCommit, toCommit) + } + logger.WarnContext(ctx, "last_scan_commit is not an ancestor of HEAD, falling back to last_scan_time", + slog.String("last_scan_commit", lastScanCommit), + slog.Time("last_scan_time", lastScanTime), + ) + } + } + + // Step 3: Execute `git log` to extract commit metadata, file status changes, and code diffs. + // Command syntax: + // `git log [--since=