fix: always skip pre-existing policy failures and clean up API surface (#3)

Remove the OnlyAppliesToNewResources guard from the pre-existing failure
skip logic so PR comments never show issues that existed before the PR.
Move isGithub into Data as IsGithubApp and switch tagging/guardrail
slices from pointers to values since go-proto returns them as non-pointers.

Author: liamcervante

pkg/vcs/comment/data.go

@@ -23,6 +23,11 @@ type Data struct {
 	// Available from runner's RunMetadata.UsageAPIEnabled.
 	UsageAPIEnabled bool
 
+	// IsGithubApp should be true when the repository is Github and the Github app (eg. the runner) is installed
+	// for this repository. It makes the comment include suggestions about using @infracost help which only works
+	// with the app in Github.
+	IsGithubApp bool
+
 	// OrgSlug is the organization slug, used to build links to Infracost Cloud settings.
 	OrgSlug string
 
@@ -79,12 +84,12 @@ type Data struct {
 
 	// TaggingPolicyResults contains the aggregated tagging evaluation results across
 	// all projects.
-	TaggingPolicyResults         []*event.TaggingPolicyResult
-	PreviousTaggingPolicyResults []*event.TaggingPolicyResult
+	TaggingPolicyResults         []event.TaggingPolicyResult
+	PreviousTaggingPolicyResults []event.TaggingPolicyResult
 
 	// GuardrailResults contains the aggregated guardrail results.
-	GuardrailResults         []*event.GuardrailResult
-	PreviousGuardrailResults []*event.GuardrailResult
+	GuardrailResults         []event.GuardrailResult
+	PreviousGuardrailResults []event.GuardrailResult
 }
 
 // ResourceSummary contains aggregated resource counts across all projects.

pkg/vcs/comment/failure_index.go

@@ -49,7 +49,7 @@ type taggingFailureIndex struct {
 	previous map[string]map[string]bool
 }
 
-func buildTaggingFailureIndex(currentResults []*event.TaggingPolicyResult, previousResults []*event.TaggingPolicyResult) taggingFailureIndex {
+func buildTaggingFailureIndex(currentResults []event.TaggingPolicyResult, previousResults []event.TaggingPolicyResult) taggingFailureIndex {
 	idx := taggingFailureIndex{
 		current:  make(map[string]map[string]bool),
 		previous: make(map[string]map[string]bool),
@@ -72,4 +72,4 @@ func buildTaggingFailureIndex(currentResults []*event.TaggingPolicyResult, previ
 	}
 
 	return idx
-}
+}

pkg/vcs/comment/fixed_issues.go

@@ -77,7 +77,7 @@ func finopsFixedIssueCounts(previousResults []*provider.FinopsPolicyResult, inde
 
 // taggingFixedIssueCounts computes fixed issue counts for tagging policies.
 // A fixed issue is an address that was failing previously but is no longer failing.
-func taggingFixedIssueCounts(results []*event.TaggingPolicyResult, index taggingFailureIndex) []FixedIssueCount {
+func taggingFixedIssueCounts(results []event.TaggingPolicyResult, index taggingFailureIndex) []FixedIssueCount {
 	var counts []FixedIssueCount
 
 	for _, result := range results {
@@ -107,4 +107,4 @@ func taggingFixedIssueCounts(results []*event.TaggingPolicyResult, index tagging
 	}
 
 	return counts
-}
+}

pkg/vcs/comment/governance.go

@@ -15,7 +15,7 @@ const (
 	GovernanceResourceLimit = 5
 )
 
-func (data *Data) processGovernance(inputs *Inputs, isGithub bool, finopsIndex, securityIndex policyFailureIndex, taggingIndex taggingFailureIndex) bool {
+func (data *Data) processGovernance(inputs *Inputs, finopsIndex, securityIndex policyFailureIndex, taggingIndex taggingFailureIndex) bool {
 	data.processPolicyResults(inputs, "FinOps policies", data.FinOpsPolicyResults, finopsIndex)
 	data.processPolicyResults(inputs, "Cloud security policies", data.SecurityPolicyResults, securityIndex)
 	data.processTaggingPolicyResults(inputs, taggingIndex)
@@ -39,7 +39,7 @@ func (data *Data) processGovernance(inputs *Inputs, isGithub bool, finopsIndex,
 		}
 	}
 
-	inputs.GovernanceSentence = formatGovernanceSentence(totalIssues, hasGuardrail, isGithub)
+	inputs.GovernanceSentence = formatGovernanceSentence(totalIssues, hasGuardrail, data.IsGithubApp)
 	return hasGuardrail
 }
 
@@ -74,7 +74,7 @@ func (data *Data) processPolicyResults(inputs *Inputs, title string, results []*
 
 		for _, resource := range result.FailingResources {
 			// Skip resources that were already failing before this PR.
-			if result.OnlyAppliesToNewResources && prevFailing[resource.Id] {
+			if prevFailing[resource.Id] {
 				continue
 			}
 
@@ -252,7 +252,7 @@ func (data *Data) processGuardrailResults(inputs *Inputs) {
 
 // formatGovernanceSentence returns a summary line about policy alignment.
 // See: dashboard/api/src/services/templates/partials/governanceOutputs.ts formatGovernanceSentence (~line 214)
-func formatGovernanceSentence(totalIssuesCount int, hasGuardrail bool, isGithub bool) string {
+func formatGovernanceSentence(totalIssuesCount int, hasGuardrail bool, isGithubApp bool) string {
 	if totalIssuesCount == 0 {
 		if hasGuardrail {
 			return ""
@@ -265,7 +265,7 @@ func formatGovernanceSentence(totalIssuesCount int, hasGuardrail bool, isGithub
 		fixing = `Consider fixing this issue, it doesn't align with your company's FinOps policies & the Well-Architected Framework.`
 	}
 
-	if isGithub {
+	if isGithubApp {
 		return fmt.Sprintf("<p>%s <b>Add a PR comment with <code>@infracost help</code> to see how you can dismiss or snooze issues and unblock your PR.</b></p>", fixing)
 	}
 
@@ -439,7 +439,7 @@ func formatTagIssues(resource event.TagPolicyResultResource) []string {
 }
 
 // formatGuardrailMessage builds the trigger description for a guardrail result.
-func formatGuardrailMessage(result *event.GuardrailResult) string {
+func formatGuardrailMessage(result event.GuardrailResult) string {
 	var parts []string
 
 	if result.Increase != nil && result.Increase.GreaterThanZero() {

pkg/vcs/comment/template.go

@@ -30,15 +30,15 @@ const truncationBuffer = 1000
 // Render executes the given template against the provided data and
 // returns the rendered string, truncating cost details if necessary
 // to fit within maxCommentSize.
-func Render(tmpl *template.Template, isGithub bool, maxCommentSize int, data Data) (string, error) {
+func Render(tmpl *template.Template, maxCommentSize int, data Data) (string, error) {
 	inputs := new(Inputs)
 	data.processProjectErrors(inputs)
 
 	finopsIndex := buildPolicyFailureIndex(data.FinOpsPolicyResults, data.PreviousFinOpsPolicyResults)
 	securityIndex := buildPolicyFailureIndex(data.SecurityPolicyResults, data.PreviousSecurityPolicyResults)
 	taggingIndex := buildTaggingFailureIndex(data.TaggingPolicyResults, data.PreviousTaggingPolicyResults)
 
-	hasGuardrail := data.processGovernance(inputs, isGithub, finopsIndex, securityIndex, taggingIndex)
+	hasGuardrail := data.processGovernance(inputs, finopsIndex, securityIndex, taggingIndex)
 	data.processFixedIssues(inputs, finopsIndex, securityIndex, taggingIndex)
 	data.processDisplayCosts(inputs, hasGuardrail)
 	data.processProjectCosts(inputs)
@@ -95,7 +95,7 @@ func truncateMiddleStr(s string, maxLen int) string {
 		return sep
 	}
 	startChars := (charsToShow + 1) / 2 // ceil
-	backChars := charsToShow / 2         // floor
+	backChars := charsToShow / 2        // floor
 	return s[:startChars] + sep + s[len(s)-backChars:]
 }
 
@@ -381,7 +381,6 @@ type CostTableEntry struct {
 	NewTotalCost string
 }
 
-
 type ProjectError struct {
 	Name  string
 	Error string

pkg/vcs/comment/template_test.go

@@ -19,26 +19,25 @@ var update = flag.Bool("update", false, "update golden files")
 func TestRender(t *testing.T) {
 	tests := []struct {
 		name           string
-		isGithub       bool
 		maxCommentSize int
 		data           Data
 		goldenFile     string
 	}{
 		{
 			name:           "minimal_empty",
-			isGithub:       true,
 			maxCommentSize: 65000,
 			data: Data{
+				IsGithubApp:      true,
 				Currency:         "USD",
 				TotalMonthlyCost: rat.Zero,
 			},
 			goldenFile: "minimal_empty.md",
 		},
 		{
 			name:           "cost_increase",
-			isGithub:       true,
 			maxCommentSize: 65000,
 			data: Data{
+				IsGithubApp:          true,
 				Currency:             "USD",
 				TotalMonthlyCost:     rat.New(250),
 				PastTotalMonthlyCost: rat.New(200),
@@ -120,9 +119,9 @@ func TestRender(t *testing.T) {
 		},
 		{
 			name:           "project_errors",
-			isGithub:       true,
 			maxCommentSize: 65000,
 			data: Data{
+				IsGithubApp:      true,
 				Currency:         "USD",
 				TotalMonthlyCost: rat.Zero,
 				Projects: []ProjectResult{
@@ -145,9 +144,9 @@ func TestRender(t *testing.T) {
 		},
 		{
 			name:           "governance_finops",
-			isGithub:       true,
 			maxCommentSize: 65000,
 			data: Data{
+				IsGithubApp:      true,
 				Currency:         "USD",
 				TotalMonthlyCost: rat.New(100),
 				RepoURL:          "https://github.com/my-org/my-repo",
@@ -178,7 +177,6 @@ func TestRender(t *testing.T) {
 		},
 		{
 			name:           "fixed_issues",
-			isGithub:       false,
 			maxCommentSize: 140000,
 			data: Data{
 				Currency:         "USD",
@@ -207,9 +205,9 @@ func TestRender(t *testing.T) {
 		},
 		{
 			name:           "preexisting_issues",
-			isGithub:       true,
 			maxCommentSize: 65000,
 			data: Data{
+				IsGithubApp:      true,
 				Currency:         "USD",
 				TotalMonthlyCost: rat.New(100),
 				OrgSlug:          "my-org",
@@ -242,15 +240,61 @@ func TestRender(t *testing.T) {
 			},
 			goldenFile: "preexisting_issues.md",
 		},
+		{
+			name:           "mixed_preexisting_and_new_issues",
+			maxCommentSize: 65000,
+			data: Data{
+				IsGithubApp:      true,
+				Currency:         "USD",
+				TotalMonthlyCost: rat.New(200),
+				RepoURL:          "https://github.com/my-org/my-repo",
+				CommitSHA:        "abc123",
+				PreviousFinOpsPolicyResults: []*provider.FinopsPolicyResult{
+					{
+						PolicySlug:                  "use-gp3",
+						PolicyName:                  "Use GP3 volumes",
+						IncludeInPullRequestComment: true,
+						FailingResources: []*provider.FinopsPolicyFailingResource{
+							{Id: "aws_ebs_volume.old"},
+						},
+					},
+				},
+				FinOpsPolicyResults: []*provider.FinopsPolicyResult{
+					{
+						PolicySlug:                  "use-gp3",
+						PolicyName:                  "Use GP3 volumes",
+						PolicyMessage:               "Use GP3 volumes instead of GP2 for better performance.",
+						IncludeInPullRequestComment: true,
+						FailingResources: []*provider.FinopsPolicyFailingResource{
+							{
+								Id:           "aws_ebs_volume.old",
+								CauseAddress: "aws_ebs_volume.old",
+								Issues: []*provider.FinopsResourceIssue{
+									{Description: "This volume uses GP2, consider upgrading to GP3"},
+								},
+							},
+							{
+								Id:           "aws_ebs_volume.new",
+								CauseAddress: "aws_ebs_volume.new",
+								Issues: []*provider.FinopsResourceIssue{
+									{Description: "This volume uses GP2, consider upgrading to GP3"},
+								},
+							},
+						},
+					},
+				},
+			},
+			goldenFile: "mixed_preexisting_and_new_issues.md",
+		},
 		{
 			name:           "guardrail_blocks",
-			isGithub:       true,
 			maxCommentSize: 65000,
 			data: Data{
+				IsGithubApp:          true,
 				Currency:             "USD",
 				TotalMonthlyCost:     rat.New(500),
 				PastTotalMonthlyCost: rat.New(100),
-				GuardrailResults: []*event.GuardrailResult{
+				GuardrailResults: []event.GuardrailResult{
 					{
 						GuardrailID:            "guardrail-1",
 						GuardrailName:          "Cost increase > $100",
@@ -299,9 +343,9 @@ func TestRender(t *testing.T) {
 		},
 		{
 			name:           "environmental_metrics",
-			isGithub:       true,
 			maxCommentSize: 65000,
 			data: Data{
+				IsGithubApp:                    true,
 				EnableEnvironmentalMetrics:      true,
 				Currency:                        "USD",
 				TotalMonthlyCost:                rat.New(300),
@@ -365,14 +409,14 @@ func TestRender(t *testing.T) {
 		},
 		{
 			name:           "governance_tagging",
-			isGithub:       true,
 			maxCommentSize: 65000,
 			data: Data{
+				IsGithubApp:      true,
 				Currency:         "USD",
 				TotalMonthlyCost: rat.New(100),
 				RepoURL:          "https://github.com/my-org/my-repo",
 				CommitSHA:        "abc123",
-				TaggingPolicyResults: []*event.TaggingPolicyResult{
+				TaggingPolicyResults: []event.TaggingPolicyResult{
 					{
 						Name:        "Require env tag",
 						TagPolicyID: "tp-1",
@@ -404,7 +448,6 @@ func TestRender(t *testing.T) {
 		},
 		{
 			name:           "rich_cost_details",
-			isGithub:       false,
 			maxCommentSize: 65000,
 			data: Data{
 				Currency:             "EUR",
@@ -542,9 +585,9 @@ func TestRender(t *testing.T) {
 		},
 		{
 			name:           "truncated_cost_details",
-			isGithub:       true,
 			maxCommentSize: 2800,
 			data: Data{
+				IsGithubApp:          true,
 				Currency:             "USD",
 				TotalMonthlyCost:     rat.New(1000),
 				PastTotalMonthlyCost: rat.New(500),
@@ -625,7 +668,7 @@ func TestRender(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := Render(DefaultTemplate, tt.isGithub, tt.maxCommentSize, tt.data)
+			got, err := Render(DefaultTemplate, tt.maxCommentSize, tt.data)
 			if err != nil {
 				t.Fatalf("Render() error: %v", err)
 			}

pkg/vcs/comment/testdata/mixed_preexisting_and_new_issues.md

@@ -0,0 +1,27 @@
+<h3>💰 Infracost report</h3>
+<p>Consider fixing this issue, it doesn't align with your company's FinOps policies & the Well-Architected Framework. <b>Add a PR comment with <code>@infracost help</code> to see how you can dismiss or snooze issues and unblock your PR.</b></p>
+
+<table>
+  <tr><td colspan="2" width="1000px">FinOps policies</td></tr>
+  
+<tr><td colspan="2" title="Failure">
+  <details >
+    <summary>
+    <b>🔴 Use GP3 volumes</b>
+  </summary><br/>
+
+Use GP3 volumes instead of GP2 for better performance.
+  </details>
+</td></tr>
+<tr><td></td><td>
+
+`aws_ebs_volume.new`
+  * This volume uses GP2, consider upgrading to GP3
+    </td></tr>
+
+  </table>
+
+<sub>
+  This comment will be updated when code changes.
+</sub>
+

pkg/vcs/github/github.go

@@ -74,7 +74,7 @@ func New(ctx context.Context, owner, repo, token string, prNumber int32, opts Op
 
 // GenerateComment renders a PR comment from the given data.
 func (g *GitHub) GenerateComment(data comment.Data) (string, error) {
-	return comment.Render(g.tmpl, true, 65000, data)
+	return comment.Render(g.tmpl, 65000, data)
 }
 
 // PostComment publishes a comment to the pull request using the given behavior.