[TE-5578] Use explicit merge-base and two-arg diff in collectDiffMetadata

Replace triple-dot syntax (baseBranch...HEAD) with an explicit
git merge-base call followed by two-arg diff (forkPoint, HEAD).
This makes fork-point resolution visible and aligns the plan path
with the backfill path which also uses two-arg diff.

Since both callers now pass exactly two arguments (base and commit),
simplify runDiffCommands from variadic refArgs to named base/commit
parameters and inline the diff calls directly.

Author: gchan

internal/git/auto_metadata.go

@@ -93,11 +93,20 @@ func collectCommitMetadata(ctx context.Context, runner GitRunner, metadata map[s
 	mergeNonEmpty(metadata, meta.ToMap())
 }
 
-// collectDiffMetadata runs diff commands against the resolved base branch
-// using triple-dot syntax (<base>...HEAD) and merges the results into the
-// metadata map.
+// collectDiffMetadata computes the merge-base between baseBranch and HEAD,
+// then runs diff commands using two-arg form (merge-base, HEAD). This is
+// equivalent to git diff baseBranch...HEAD but makes the fork-point
+// resolution explicit and uses the same two-arg diff form as the backfill
+// path.
 func collectDiffMetadata(ctx context.Context, runner GitRunner, baseBranch string, metadata map[string]string) {
-	diffs := runDiffCommands(ctx, runner, false, baseBranch+"...HEAD")
+	forkPoint, err := runner.Output(ctx, "merge-base", baseBranch, "HEAD")
+	if err != nil {
+		debug.Printf("Warning: git merge-base failed: %v", err)
+		return
+	}
+	forkPoint = strings.TrimSpace(forkPoint)
+
+	diffs := runDiffCommands(ctx, runner, false, forkPoint, "HEAD")
 	mergeNonEmpty(metadata, diffs.ToMap())
 }
 

internal/git/auto_metadata_test.go

@@ -159,12 +159,13 @@ func TestCollectPlanMetadata_HappyPath(t *testing.T) {
 
 	runner := &FakeGitRunner{
 		Responses: map[string]string{
-			fmt.Sprintf("log -1 --format=%s", MetadataFormat):   gitLogOutput,
-			"diff --no-ext-diff --name-only origin/main...HEAD": "file1.go\nfile2.go\n",
-			"diff --no-ext-diff --numstat origin/main...HEAD":   "10\t5\tfile1.go\n3\t0\tfile2.go\n",
-			"diff --no-ext-diff origin/main...HEAD":             "diff --git a/file1.go b/file1.go\n",
-			"diff --no-ext-diff --raw origin/main...HEAD":       ":100644 100644 aaa bbb M\tfile1.go\n",
-			"branch --show-current":                             "feature/my-branch\n",
+			fmt.Sprintf("log -1 --format=%s", MetadataFormat): gitLogOutput,
+			"merge-base origin/main HEAD":                     "aaa000\n",
+			"diff --no-ext-diff --name-only aaa000 HEAD":      "file1.go\nfile2.go\n",
+			"diff --no-ext-diff --numstat aaa000 HEAD":        "10\t5\tfile1.go\n3\t0\tfile2.go\n",
+			"diff --no-ext-diff aaa000 HEAD":                  "diff --git a/file1.go b/file1.go\n",
+			"diff --no-ext-diff --raw aaa000 HEAD":            ":100644 100644 aaa bbb M\tfile1.go\n",
+			"branch --show-current":                           "feature/my-branch\n",
 		},
 	}
 
@@ -317,11 +318,12 @@ func TestCollectPlanMetadata_LogFails(t *testing.T) {
 	runner := &FakeGitRunner{
 		Responses: map[string]string{
 			// git log missing -> will error
-			"diff --no-ext-diff --name-only origin/main...HEAD": "file1.go\n",
-			"diff --no-ext-diff --numstat origin/main...HEAD":   "10\t5\tfile1.go\n",
-			"diff --no-ext-diff origin/main...HEAD":             "diff text\n",
-			"diff --no-ext-diff --raw origin/main...HEAD":       ":100644 raw\n",
-			"branch --show-current":                             "feature\n",
+			"merge-base origin/main HEAD":                "aaa000\n",
+			"diff --no-ext-diff --name-only aaa000 HEAD": "file1.go\n",
+			"diff --no-ext-diff --numstat aaa000 HEAD":   "10\t5\tfile1.go\n",
+			"diff --no-ext-diff aaa000 HEAD":             "diff text\n",
+			"diff --no-ext-diff --raw aaa000 HEAD":       ":100644 raw\n",
+			"branch --show-current":                      "feature\n",
 		},
 	}
 
@@ -484,12 +486,13 @@ func TestCollectPlanMetadata_EmptyDiffOutput(t *testing.T) {
 
 	runner := &FakeGitRunner{
 		Responses: map[string]string{
-			fmt.Sprintf("log -1 --format=%s", MetadataFormat):   gitLogOutput,
-			"diff --no-ext-diff --name-only origin/main...HEAD": "\n",
-			"diff --no-ext-diff --numstat origin/main...HEAD":   "\n",
-			"diff --no-ext-diff origin/main...HEAD":             "\n",
-			"diff --no-ext-diff --raw origin/main...HEAD":       "\n",
-			"branch --show-current":                             "main\n",
+			fmt.Sprintf("log -1 --format=%s", MetadataFormat): gitLogOutput,
+			"merge-base origin/main HEAD":                     "aaa000\n",
+			"diff --no-ext-diff --name-only aaa000 HEAD":      "\n",
+			"diff --no-ext-diff --numstat aaa000 HEAD":        "\n",
+			"diff --no-ext-diff aaa000 HEAD":                  "\n",
+			"diff --no-ext-diff --raw aaa000 HEAD":            "\n",
+			"branch --show-current":                           "main\n",
 		},
 	}
 

internal/git/diff.go

@@ -133,44 +133,32 @@ func extractCommitDiffs(
 	return runDiffCommands(ctx, runner, skipDiffs, fp.Base, commit), nil
 }
 
-// runDiffCommands runs the 4 git diff commands against the given ref args
-// and returns the results as a CommitDiffs struct. The refArgs are passed
-// directly to git diff (e.g. ["base", "commit"] for two-arg form, or
-// ["base...HEAD"] for triple-dot form). Each command is independent; failures
-// log a debug warning and leave the field empty.
-func runDiffCommands(ctx context.Context, runner GitRunner, skipDiffs bool, refArgs ...string) CommitDiffs {
-	// diffArgs builds the full argument list for a git diff command.
-	// flags are inserted between "diff --no-ext-diff" and the ref args.
-	diffArgs := func(flags ...string) []string {
-		args := make([]string, 0, 2+len(flags)+len(refArgs))
-		args = append(args, "diff", "--no-ext-diff")
-		args = append(args, flags...)
-		args = append(args, refArgs...)
-		return args
-	}
-
+// runDiffCommands runs the 4 git diff commands between base and commit,
+// returning the results as a CommitDiffs struct. Each command is independent;
+// failures log a debug warning and leave the field empty.
+func runDiffCommands(ctx context.Context, runner GitRunner, skipDiffs bool, base, commit string) CommitDiffs {
 	var diffs CommitDiffs
 
-	if out, err := runner.Output(ctx, diffArgs("--name-only")...); err == nil {
+	if out, err := runner.Output(ctx, "diff", "--no-ext-diff", "--name-only", base, commit); err == nil {
 		diffs.FilesChanged = strings.TrimRight(out, "\n")
 	} else {
 		debug.Printf("Warning: diff --name-only failed: %v", err)
 	}
 
-	if out, err := runner.Output(ctx, diffArgs("--numstat")...); err == nil {
+	if out, err := runner.Output(ctx, "diff", "--no-ext-diff", "--numstat", base, commit); err == nil {
 		diffs.DiffStat = strings.TrimRight(out, "\n")
 	} else {
 		debug.Printf("Warning: diff --numstat failed: %v", err)
 	}
 
 	if !skipDiffs {
-		if out, err := runner.Output(ctx, diffArgs()...); err == nil {
+		if out, err := runner.Output(ctx, "diff", "--no-ext-diff", base, commit); err == nil {
 			diffs.GitDiff = strings.TrimRight(out, "\n")
 		} else {
 			debug.Printf("Warning: diff failed: %v", err)
 		}
 
-		if out, err := runner.Output(ctx, diffArgs("--raw")...); err == nil {
+		if out, err := runner.Output(ctx, "diff", "--no-ext-diff", "--raw", base, commit); err == nil {
 			diffs.GitDiffRaw = strings.TrimRight(out, "\n")
 		} else {
 			debug.Printf("Warning: diff --raw failed: %v", err)