Preserve historical SHA pins during branch normalization

internal/pin/normalization_test.go

@@ -74,6 +74,10 @@ func TestPlanNormalizationPrunesStaleTargetsInOneRun(t *testing.T) {
 					tags = httpmock.TagListResponse(tt.tag, headSHA)
 				}
 				reg.Register(httpmock.REST("GET", `repos/owner/action/tags`), httpmock.JSONResponse(tags))
+			}
+			// Branch normalization requires the branch head to equal the pin,
+			// even when the lockfile already records the containing branch.
+			if !tt.recorded || tt.tag == "" {
 				reg.Register(httpmock.REST("GET", `repos/owner/action/git/ref/heads/main`),
 					httpmock.JSONResponse(map[string]any{
 						"ref": "refs/heads/main", "object": map[string]any{"sha": headSHA, "type": "commit"},

internal/pin/plan.go

@@ -638,8 +638,12 @@ func narrowVerifiedEntries(ctx context.Context, entries []Entry, opts PlanOption
 		}
 		if parserlock.IsFullSha(e.Ref) {
 			newRef := e.Tag
-			if newRef == "" {
-				newRef = e.OnBranch
+			if newRef == "" && e.OnBranch != "" && opts.Resolver != nil {
+				// A containing branch cannot identify an older pinned commit.
+				owner, repo := splitNWO(e.NWO)
+				if head, ok := opts.Resolver.GetBranchHead(ctx, owner, repo, e.OnBranch); ok && strings.EqualFold(head.SHA, e.SHA) {
+					newRef = e.OnBranch
+				}
 			}
 			if newRef == "" {
 				continue

internal/pin/plan_test.go

@@ -418,7 +418,13 @@ func TestNarrowVerifiedEntries_StickyPrecision(t *testing.T) {
 	})
 
 	t.Run("SHA metadata repairs to branch", func(t *testing.T) {
-		tagger, _ := newTagger(t)
+		tagger, reg := newTagger(t)
+		reg.Register(httpmock.REST("GET", `repos/actions/checkout/git/ref/heads/main`),
+			httpmock.JSONResponse(map[string]any{
+				"ref": "refs/heads/main", "object": map[string]any{"sha": sha, "type": "commit"},
+			}))
+		resolver, err := resolve.New("github.com", pinpool.New(2, nil), resolve.WithTransport(reg))
+		require.NoError(t, err)
 		report := fastPathReport(sha)
 		report.Inventory[0].Dep.Branch = "main"
 		report.ActionRefs = []parserlock.ActionRef{{
@@ -427,7 +433,7 @@ func TestNarrowVerifiedEntries_StickyPrecision(t *testing.T) {
 			Ref:   sha,
 		}}
 
-		result, err := planWorkflow(context.Background(), report, PlanOptions{Tagger: tagger}, func(string) {})
+		result, err := planWorkflow(context.Background(), report, PlanOptions{Tagger: tagger, Resolver: resolver}, func(string) {})
 		require.NoError(t, err)
 
 		require.Len(t, result.entries, 1)
@@ -506,6 +512,74 @@ func TestNarrowVerifiedEntries_StickyPrecision(t *testing.T) {
 	})
 }
 
+func TestPlanPreservesDifferentHistoricalSHAsOnSameBranch(t *testing.T) {
+	const headSHA = "cccccccccccccccccccccccccccccccccccccccc"
+	shas := []string{
+		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+		"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+	}
+	for _, tt := range []struct {
+		name   string
+		locked int
+	}{
+		{name: "fresh lockfile"},
+		{name: "existing lockfile", locked: 2},
+		{name: "new workflow with existing lockfile", locked: 1},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			reg := &httpmock.Registry{}
+			defer reg.Verify(t)
+			reg.Register(httpmock.REST("GET", `repos/owner/action/git/ref/heads/main`),
+				httpmock.JSONResponse(map[string]any{
+					"ref": "refs/heads/main", "object": map[string]any{"sha": headSHA, "type": "commit"},
+				}))
+			if tt.locked < len(shas) {
+				reg.Register(httpmock.REST("GET", `repos/owner/action$`),
+					httpmock.JSONResponse(map[string]any{"default_branch": "main"}))
+				reg.Register(httpmock.REST("GET", `repos/owner/action/tags`),
+					httpmock.JSONResponse(httpmock.TagListResponse()))
+				for _, sha := range shas[tt.locked:] {
+					reg.Register(httpmock.REST("GET", `repos/owner/action/compare/`+sha+`\.\.\.`+headSHA),
+						httpmock.JSONResponse(httpmock.CompareAncestorResponse(sha)))
+				}
+			}
+			pool := pinpool.New(2, nil)
+			resolver, err := resolve.New("github.com", pool, resolve.WithTransport(reg))
+			require.NoError(t, err)
+			report := &checks.Report{}
+			for i, sha := range shas {
+				d := dep.Dependency{Hostname: "github.com", NWO: "owner/action", Ref: sha, SHA: sha, Branch: "main"}
+				resolver.SeedFromLockfile([]dep.Dependency{d})
+				wr := checks.WorkflowReport{
+					Path:       ".github/workflows/" + sha + ".yml",
+					ActionRefs: []parserlock.ActionRef{{Owner: "owner", Repo: "action", Ref: sha}},
+				}
+				if i < tt.locked {
+					wr.Inventory = []checks.InventoryEntry{{Dep: d, File: wr.Path, Direct: true}}
+				} else {
+					wr.Findings = []checks.Finding{{Category: checks.NotPinned, Severity: checks.SeverityError}}
+				}
+				report.Workflows = append(report.Workflows, wr)
+			}
+
+			record, err := Plan(context.Background(), report, PlanOptions{
+				Resolver: resolver, Tagger: tag.NewListerForTest(t, reg), Pool: pool,
+			})
+			require.NoError(t, err)
+			require.Len(t, record.Entries, len(shas))
+			for i, entry := range record.Entries {
+				assert.Equal(t, shas[i], entry.Ref)
+				assert.Equal(t, shas[i], entry.SHA)
+				assert.Equal(t, "main", entry.OnBranch)
+			}
+			require.Len(t, record.Workflows, len(shas))
+			for _, wp := range record.Workflows {
+				assert.Empty(t, wp.Rewrites)
+			}
+		})
+	}
+}
+
 func TestPlanNormalizationRespectsLockedTarget(t *testing.T) {
 	const (
 		oldSHA  = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

internal/resolve/reverse_lookup.go

@@ -252,8 +252,9 @@ type LookupIssue struct {
 
 // ReverseLookup leaves symbolic refs unchanged and reverse-lookups bare SHA
 // refs via DiscoverContaining. For bare SHAs it populates dep.Tag and dep.Branch
-// and computes the canonical @ref. The ref priority is:
-// tag (semver-ish release) > protected branch > default branch > any branch.
+// and computes the canonical @ref. Exact tags take precedence over branch
+// heads. A containing branch remains metadata when its head differs from SHA,
+// so historical pins on the same branch keep distinct refs.
 // When the canonical ref differs from dep.Ref the change is recorded in
 // the returned rewrites map and dep.Ref is updated in place.
 //
@@ -300,7 +301,9 @@ func (r *Resolver) ReverseLookup(ctx context.Context, deps []dep.Dependency) (ma
 			newRef = branch
 		}
 		if newRef == "" {
-			newRef = branch
+			if head, ok := r.GetBranchHead(ctx, owner, repo, branch); ok && strings.EqualFold(head.SHA, d.SHA) {
+				newRef = branch
+			}
 		}
 		if newRef == "" || newRef == d.Ref {
 			continue

internal/resolve/reverse_lookup_test.go

@@ -8,8 +8,62 @@ import (
 	"github.com/github/gh-actions-lock/internal/ghapi"
 	"github.com/github/gh-actions-lock/internal/ghapi/httpmock"
 	"github.com/github/gh-actions-lock/internal/pinpool"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+func TestReverseLookup_ContainingBranchDoesNotIdentifyHistoricalCommit(t *testing.T) {
+	const (
+		oldSHA  = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+		headSHA = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+	)
+	for _, tt := range []struct {
+		name    string
+		sha     string
+		tag     string
+		wantRef string
+	}{
+		{name: "historical SHA stays distinct", sha: oldSHA, wantRef: oldSHA},
+		{name: "branch head can use branch name", sha: headSHA, wantRef: "main"},
+		{name: "exact tag identifies historical commit", sha: oldSHA, tag: "v1.0.0", wantRef: "v1.0.0"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			reg := &httpmock.Registry{}
+			defer reg.Verify(t)
+			reg.Register(httpmock.REST("GET", `repos/owner/action$`),
+				httpmock.JSONResponse(map[string]any{"default_branch": "main"}))
+			reg.Register(httpmock.REST("GET", `repos/owner/action/git/ref/heads/main`),
+				httpmock.JSONResponse(map[string]any{
+					"ref": "refs/heads/main", "object": map[string]any{"sha": headSHA, "type": "commit"},
+				}))
+			if tt.sha != headSHA {
+				reg.Register(httpmock.REST("GET", `repos/owner/action/compare/`+oldSHA+`\.\.\.`+headSHA),
+					httpmock.JSONResponse(httpmock.CompareAncestorResponse(oldSHA)))
+			}
+			tags := httpmock.TagListResponse()
+			if tt.tag != "" {
+				tags = httpmock.TagListResponse(tt.tag, tt.sha)
+			}
+			reg.Register(httpmock.REST("GET", `repos/owner/action/tags`), httpmock.JSONResponse(tags))
+			resolver, err := New("github.com", pinpool.New(2, nil), WithTransport(reg))
+			require.NoError(t, err)
+			deps := []dep.Dependency{{NWO: "owner/action", Ref: tt.sha, SHA: tt.sha}}
+
+			rewrites, issues, err := resolver.ReverseLookup(context.Background(), deps)
+			require.NoError(t, err)
+			assert.Empty(t, issues)
+			assert.Equal(t, tt.wantRef, deps[0].Ref)
+			assert.Equal(t, "main", deps[0].Branch)
+			assert.Equal(t, tt.tag, deps[0].Tag)
+			if tt.wantRef == tt.sha {
+				assert.Empty(t, rewrites)
+			} else {
+				assert.Equal(t, map[string]string{"owner/action@" + tt.sha: "owner/action@" + tt.wantRef}, rewrites)
+			}
+		})
+	}
+}
+
 func TestFindContainingBranch_ExactHeadMatch(t *testing.T) {
 	reg := &httpmock.Registry{}
 	r, err := New("test.com", pinpool.New(1, nil), WithTransport(reg))