fix

Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>

Author: MeNsaaH

pkg/msg/message.go

@@ -196,6 +196,7 @@ func (m *Message) BuildComment(
 	// Footer warning for split comments
 	splitCommentFooter := "\n\n> **Warning**: Output length greater than maximum allowed comment size. Continued in next comment"
 	// Header for continued comments
+	// this is written from the VCS PostMessage/UpdateMessage function. So, we need to account for it here
 	continuedHeader := fmt.Sprintf("%s> Continued from previous [comment](%s)\n\n", header, prLinkTemplate)
 	// Max content length for each chunk (accounting for continuedHeader)
 	maxContentLength := maxCommentLength - len(continuedHeader)
@@ -211,13 +212,12 @@ func (m *Message) BuildComment(
 	// and starting new chunks with continuedHeader after the first
 	appendChunk := func(appHeader string) {
 		if sb.Len() > 0 {
-			currentContent := sb.String()
 			// Only add splitCommentFooter if there's enough space
-			if len(currentContent)+len(splitCommentFooter) <= maxContentLength {
+			if (contentLength)+len(splitCommentFooter) <= maxContentLength {
 				sb.WriteString(splitCommentFooter)
 			} else {
 				// Create a truncated copy of splitCommentFooter to make room for it
-				availableSpace := maxContentLength - len(currentContent)
+				availableSpace := maxContentLength - contentLength
 				if availableSpace > 0 {
 					truncatedFooter := splitCommentFooter[:availableSpace]
 					sb.WriteString(truncatedFooter)
@@ -226,11 +226,11 @@ func (m *Message) BuildComment(
 			comments = append(comments, sb.String())
 			sb.Reset()
 			sb.WriteString(header)
-			sb.WriteString(appHeader)
 			// continuedHeader contains both the header and the comment about the continued fromprevious comment
 			// header is written here but the rest of the content is written from the vcs PostMessage/UpdateMessage function
 			// that's why we're setting the contentLength to the length of the continuedHeader
 			contentLength = len(continuedHeader)
+			sb.WriteString(appHeader)
 			contentLength += len(appHeader)
 
 		}
@@ -341,13 +341,16 @@ func (m *Message) BuildComment(
 			}
 		}
 
-		// Don't write if there's no more apps to write. An unclosed details tag wouldn't cause an issue
+		// Don't split if there's no more apps to write. An unclosed details tag wouldn't cause an issue
 		// unless there's more contents to write
+		closingDetailsTag := "\n</details>\n"
 		if appIndex < len(names)-1 {
-			closingDetailsTag := "\n</details>\n"
 			if contentLength+len(closingDetailsTag) > maxContentLength {
 				appendChunk(appHeader)
 			} // Close the app details block
+		}
+		// if there's space to write the closingTag, write it
+		if contentLength+len(closingDetailsTag) < maxContentLength {
 			sb.WriteString(closingDetailsTag)
 			contentLength += len(closingDetailsTag)
 		}

pkg/msg/message_test.go

@@ -414,6 +414,7 @@ func TestBuildComment_Deep(t *testing.T) {
 			if strings.Contains(comments[i], "> **Warning**: Output length greater than maximum allowed comment size. Continued in next comment") {
 				foundSplitWarnings++
 			}
+			assert.Less(t, len(comments[i]), 500)
 		}
 		assert.Greater(t, foundSplitWarnings, 0, "Should have at least one split warning")
 		// Last comment should have footer
@@ -552,7 +553,7 @@ func TestBuildComment_Deep(t *testing.T) {
 		m := NewMessage("test", 1, 2, fakeVCS)
 		m.AddNewApp(ctx, "small-limit-app")
 		m.AddToAppMessage(ctx, "small-limit-app", Result{State: pkg.StateSuccess, Summary: "test summary", Details: "test details"})
-		comments := m.BuildComment(ctx, time.Now(), "sha", "", false, "id", 1, 1, 100, "prlink")
+		comments := m.BuildComment(ctx, time.Now(), "sha", "", false, "id", 1, 1, 200, "prlink")
 		require.Greater(t, len(comments), 1)
 		// Should have multiple comments due to very small limit
 		assert.Contains(t, comments[0], "# Kubechecks id Report")
@@ -569,6 +570,9 @@ func TestBuildComment_Deep(t *testing.T) {
 			if strings.Contains(comment, "small-limit-app") {
 				foundAppName = true
 			}
+			// With the current implementation, comments can be up to maxCommentLength (100)
+			// Allow for some flexibility in the splitting logic
+			assert.LessOrEqual(t, len(comment), 200, "Comment should not exceed maxCommentLength")
 			if strings.Contains(comment, "test summary") {
 				foundSummary = true
 			}
@@ -1147,6 +1151,24 @@ func TestSplitContentPreservingCodeBlocks_Debug(t *testing.T) {
 	t.Logf("Inside code block: %v", codeBlockMarkers%2 == 1)
 }
 
+func TestBuildComment_NoTrailingDetailsTag(t *testing.T) {
+	m := NewMessage("test", 1, 2, fakeEmojiable{":ok:"})
+	ctx := context.TODO()
+	m.AddNewApp(ctx, "app1")
+	m.AddToAppMessage(ctx, "app1", Result{State: pkg.StateSuccess, Summary: "all good", Details: "details"})
+	comments := m.BuildComment(ctx, time.Now(), "sha", "", false, "id", 1, 1, 1000, "prlink")
+	require.Len(t, comments, 1)
+	output := comments[0]
+
+	// The output should not end with </details> followed by the footer
+	footer := "<small> _Done. CommitSHA: sha_ <small>"
+	assert.True(t, strings.HasSuffix(output, footer+"\n"), "Output should end with the footer")
+	// There should not be a trailing </details> before the footer
+	lastDetailsIdx := strings.LastIndex(output, "</details>")
+	footerIdx := strings.LastIndex(output, footer)
+	assert.True(t, lastDetailsIdx < footerIdx, "No trailing </details> after last app block before footer")
+}
+
 // Helper function for min
 func min(a, b int) int {
 	if a < b {