fix(internal/conventionalcommits): relax footer regex to allow line broke footers to be properly recognized (#2388)

zhumin8 · web-flow · commit dcbe0635209a · 2025-09-29T14:35:50.000-04:00
The original logic expect footer to follow `key: value` (exactly one space before value) format as conventionalcommit specs. For multi-line footers, when parsing ([logic](https://github.com/googleapis/librarian/blob/9b28aa4a3619579dfa59c430afa612589b353598/internal/conventionalcommits/conventional_commits.go#L336)), the line content "Source-Link:" would fail the regex match and thus not properly recognized as footer, and gets appended to the value of the previous footer. By relaxing the regex a bit to allow any number of spaces before "value" in footer, the multiline footer can be recognized. note: this format below from owlbot PR ([example](googleapis/google-cloud-python@34a7916)) is technically not valid footer according to conventionalcommits.org spec [here](https://www.conventionalcommits.org/en/v1.0.0/#:~:text=Each%20footer%20MUST%20consist%20of%20a%20word%20token%2C%20followed%20by%20either%20a%20%3A%3Cspace%3E%20or%20%3Cspace%3E%23%20separator%2C%20followed%20by%20a%20string%20value). But we are relaxing to allow it for onboarding libraries. ``` Source-Link: googleapis/googleapis@d300b15 ``` This should solve the first issue causing this bug, see [comment](#2080 (comment)) For #2080
diff --git a/internal/conventionalcommits/conventional_commits.go b/internal/conventionalcommits/conventional_commits.go
@@ -38,9 +38,9 @@ var (
 	commitRegex = regexp.MustCompile(`^(?P<type>\w+)(?:\((?P<scope>.*)\))?(?P<breaking>!)?:\s(?P<description>.*)`)
 	// footerRegex defines the format for a conventional commit footer.
 	// A footer key consists of letters and hyphens, or is the "BREAKING CHANGE"
-	// literal. The key is followed by ": " and then the value.
+	// literal. The key is followed by ":" and then the value.
 	// e.g., "Reviewed-by: G. Gemini" or "BREAKING CHANGE: an API was changed".
-	footerRegex     = regexp.MustCompile(`^([A-Za-z-]+|` + breakingChangeKey + `):\s(.*)`)
+	footerRegex     = regexp.MustCompile(`^([A-Za-z-]+|` + breakingChangeKey + `):(.*)`)
 	sourceLinkRegex = regexp.MustCompile(`^\[googleapis\/googleapis@(?P<shortSHA>.*)\]\(https:\/\/github\.com\/googleapis\/googleapis\/commit\/(?P<sha>.*)\)$`)
 	// libraryIDRegex extracts the libraryID from the commit message in a generation PR.
 	// For a generation PR, each commit is expected to have the libraryID in brackets
@@ -298,6 +298,8 @@ func parseHeader(headerLine string) (*parsedHeader, bool) {
 }
 
 // separateBodyAndFooters splits the lines after the header into body and footer sections.
+// It identifies the footer section by looking for a blank line followed by a
+// line that matches the conventional commit footer format.
 func separateBodyAndFooters(lines []string) (bodyLines, footerLines []string) {
 	inFooterSection := false
 	for i, line := range lines {
diff --git a/internal/conventionalcommits/conventional_commits_test.go b/internal/conventionalcommits/conventional_commits_test.go
@@ -693,3 +693,85 @@ func TestConventionalCommit_MarshalJSON(t *testing.T) {
 		t.Errorf("MarshalJSON() mismatch (-want +got):\n%s", diff)
 	}
 }
+
+func TestParseFooters(t *testing.T) {
+	for _, test := range []struct {
+		name           string
+		footerLines    []string
+		wantFooters    map[string]string
+		wantIsBreaking bool
+	}{
+		{
+			name: "single footer",
+			footerLines: []string{
+				"Reviewed-by: G. Gemini",
+			},
+			wantFooters: map[string]string{
+				"Reviewed-by": "G. Gemini",
+			},
+		},
+		{
+			name: "multiple footers",
+			footerLines: []string{
+				"Reviewed-by: G. Gemini",
+				"Co-authored-by: Another Person <another@person.com>",
+			},
+			wantFooters: map[string]string{
+				"Reviewed-by":    "G. Gemini",
+				"Co-authored-by": "Another Person <another@person.com>",
+			},
+		},
+		{
+			name: "multiline footer",
+			footerLines: []string{
+				"BREAKING CHANGE: something broke",
+				"  and it was a big deal",
+			},
+			wantFooters: map[string]string{
+				"BREAKING CHANGE": "something broke\n  and it was a big deal",
+			},
+			wantIsBreaking: true,
+		},
+		{
+			name: "empty lines",
+			footerLines: []string{
+				"Reviewed-by: G. Gemini",
+				"",
+				"",
+				"Co-authored-by: Another Person <another@person.com>",
+			},
+			wantFooters: map[string]string{
+				"Reviewed-by":    "G. Gemini",
+				"Co-authored-by": "Another Person <another@person.com>",
+			},
+		},
+		{
+			name: "multi-line footers with key on one line, value on the next",
+			footerLines: []string{
+				"PiperOrigin-RevId: 123456",
+				"",
+				"Source-Link:",
+				"",
+				"googleapis/googleapis@a12b345",
+				"",
+				"",
+				"Copy-Tag:",
+				"eyJwIjoic",
+			},
+			wantFooters: map[string]string{
+				"PiperOrigin-RevId": "123456",
+				"Source-Link":       "\ngoogleapis/googleapis@a12b345",
+				"Copy-Tag":          "\neyJwIjoic"},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			gotFooters, gotIsBreaking := parseFooters(test.footerLines)
+			if diff := cmp.Diff(test.wantFooters, gotFooters); diff != "" {
+				t.Errorf("parseFooters() footers mismatch (-want +got):%s", diff)
+			}
+			if gotIsBreaking != test.wantIsBreaking {
+				t.Errorf("parseFooters() isBreaking = %v, want %v", gotIsBreaking, test.wantIsBreaking)
+			}
+		})
+	}
+}
