CLOUD-1721 Add TFBUDDY_DELETE_OLD_COMMENTS feature (#53)

* CLOUD-1721 Add TFBUDDY_DELETE_OLD_COMMENTS feature

Signed-off-by: Ihor Horak <ihor.horak@zapier.com>

Author: horakihor

README.md

@@ -13,6 +13,8 @@ apply-before-merge workflow.
 This tool provides a server function that processes webhooks from Gitlab/Github, triggers a Run in TFC for Merge/Pull Requests
 and then passes status updates of those Runs back to the Merge/Pull Request in the form of comments.
 
+For MRs with multiple workspaces, TFBuddy tracks each workspace independently and can automatically clean up old plan/apply comments, keeping only the most recent one per workspace and action type. Set `TFBUDDY_DELETE_OLD_COMMENTS` to enable this.
+
 
 ### Architecture
 

docs/architecture.md

@@ -16,6 +16,10 @@ If you're happy with the plan you can issue a apply command `tfc apply` or if yo
 
 Once the apply completes TF Buddy will update the PR indicating what was changed and if there was any errors.
 
+#### Comment Cleanup
+
+When `TFBUDDY_DELETE_OLD_COMMENTS` is set, TFBuddy automatically cleans up old discussion threads to keep MRs/PRs readable. Each comment is tagged with an invisible HTML marker identifying its workspace and action (plan or apply). When a new run completes, TFBuddy deletes older discussions that match the same workspace and action, keeping only the latest one. This is workspace-scoped: if your MR triggers runs in multiple workspaces, each workspace retains its own most recent plan and apply comment independently. Previous run URLs are collected into a collapsible "Previous TFC Urls" table on the latest comment.
+
 Example of how an error is reported
 
 ![error](img/error.png)

docs/usage.md

@@ -55,6 +55,11 @@ env:
   # Optional: fail CI if Sentinel policy checks soft-fail on plan (default: disabled)
   # Set to "true" to mark the plan commit status as failed when policies soft-fail
   TFBUDDY_FAIL_CI_ON_SENTINEL_SOFT_FAIL: "false"
+  # Optional: enable automatic cleanup of old discussion comments per workspace and action.
+  # When enabled ("true"), TFBuddy deletes previous plan/apply discussions for the same
+  # workspace, keeping only the most recent one. Discussions for other workspaces or
+  # actions are preserved. Set to "false" or remove to disable.
+  TFBUDDY_DELETE_OLD_COMMENTS: "true"
 ```
 
 For sensitive environment variables use `secrets.envs` which can contain a list of key/value pairs

localdev/manifests/deployment.yaml

@@ -28,6 +28,8 @@ spec:
             - name: TFBUDDY_OTEL_COLLECTOR_PORT
               value: "4317"
             - name: TFBUDDY_ALLOW_AUTO_MERGE
+              value: "false"
+            - name: TFBUDDY_DELETE_OLD_COMMENTS
               value: "true"
             - name: TFBUDDY_FAIL_CI_ON_SENTINEL_SOFT_FAIL
               value: "true"

pkg/comment_actions/parsing.go

@@ -38,6 +38,16 @@ func ParseCommentCommand(noteBody string) (*CommentOpts, error) {
 		return nil, ErrNoNotePassed
 	}
 
+	switch words[0] {
+	case "tfc":
+		// valid agent, continue parsing
+	case "terraform", "atlantis":
+		log.Debug().Str("comment", words[0]).Msg("Use tfc to interact with tfbuddy")
+		return nil, ErrOtherTFTool
+	default:
+		return nil, ErrNotTFCCommand
+	}
+
 	opts := &CommentOpts{
 		TriggerOpts: &tfc_trigger.TFCTriggerOptions{},
 	}
@@ -47,14 +57,6 @@ func ParseCommentCommand(noteBody string) (*CommentOpts, error) {
 		return nil, ErrPermanent
 	}
 
-	if opts.Args.Agent == "terraform" || opts.Args.Agent == "atlantis" {
-		log.Debug().Str("comment", opts.Args.Agent).Msg("Use tfc to interact with tfbuddy")
-		return nil, ErrOtherTFTool
-	}
-	if opts.Args.Agent != "tfc" {
-		return nil, ErrNotTFCCommand
-	}
-
 	opts.TriggerOpts.Action = tfc_trigger.CheckTriggerAction(opts.Args.Command)
 	if opts.TriggerOpts.Action == tfc_trigger.InvalidAction {
 		return nil, ErrInvalidAction

pkg/comment_formatter/tfc_status_update.go

@@ -11,6 +11,7 @@ import (
 	"github.com/zapier/tfbuddy/pkg/runstream"
 	"github.com/zapier/tfbuddy/pkg/terraform_plan"
 	"github.com/zapier/tfbuddy/pkg/tfc_api"
+	"github.com/zapier/tfbuddy/pkg/utils"
 )
 
 func getProperApplyText(rmd runstream.RunMetadata, wsName string) string {
@@ -132,7 +133,7 @@ func FormatRunStatusCommentBody(tfc tfc_api.ApiClient, run *tfe.Run, rmd runstre
 		rmd.GetAction(),
 		run.Status,
 		run.ID, runUrl,
-	)
+	) + "\n" + utils.FormatTFBuddyMarker(wsName, rmd.GetAction())
 
 	return extraInfo, topLevelNoteBody, resolveDiscussion
 

pkg/mocks/helpers.go

@@ -168,7 +168,7 @@ func (ts *TestSuite) InitTestSuite() {
 	ts.MockGitClient.EXPECT().GetMergeRequestModifiedFiles(gomock.Any(), ts.MetaData.MRIID, ts.MetaData.ProjectNameNS).Return([]string{"main.tf"}, nil).AnyTimes()
 	ts.MockGitClient.EXPECT().GetRepoFile(gomock.Any(), ts.MetaData.ProjectNameNS, ".tfbuddy.yaml", ts.MetaData.SourceBranch).Return(ts.MetaData.TFBuddyConfig, nil).AnyTimes()
 	ts.MockGitClient.EXPECT().CloneMergeRequest(gomock.Any(), ts.MetaData.ProjectNameNS, gomock.Any(), gomock.Any()).Return(ts.MockGitRepo, nil).AnyTimes()
-	ts.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), ts.MetaData.MRIID, ts.MetaData.ProjectNameNS, &RegexMatcher{regex: regexp.MustCompile("Starting TFC apply for Workspace: `([A-z\\-]){1,}/([A-z\\-]){1,}`.")}).Return(ts.MockGitDisc, nil).AnyTimes()
+	ts.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), ts.MetaData.MRIID, ts.MetaData.ProjectNameNS, &RegexMatcher{regex: regexp.MustCompile(`Starting TFC apply for Workspace: ` + "`" + `([A-Za-z0-9\-]){1,}/([A-Za-z0-9\-]){1,}` + "`" + `\.\n<!-- tfbuddy:ws=.+:action=.+ -->`)}).Return(ts.MockGitDisc, nil).AnyTimes()
 
 	ts.MockApiClient.EXPECT().GetWorkspaceByName(gomock.Any(), gomock.Any(), gomock.Any()).Return(&tfe.Workspace{ID: "service-tfbuddy"}, nil).AnyTimes()
 	ts.MockApiClient.EXPECT().GetTagsByQuery(gomock.Any(), gomock.Any(), "tfbuddylock").AnyTimes()

pkg/mocks/mock_vcs.go

@@ -585,7 +585,8 @@ func (t *TFCTrigger) triggerRunForWorkspace(ctx context.Context, cfgWS *TFCWorks
 	// create a new Merge Request discussion thread where status updates will be nested
 	disc, err := t.gl.CreateMergeRequestDiscussion(ctx, mr.GetInternalID(),
 		t.GetProjectNameWithNamespace(),
-		fmt.Sprintf("Starting TFC %v for Workspace: `%s/%s`.", t.GetAction(), org, wsName),
+		fmt.Sprintf("Starting TFC %v for Workspace: `%s/%s`.\n", t.GetAction(), org, wsName)+
+			utils.FormatTFBuddyMarker(wsName, t.GetAction().String()),
 	)
 	if err != nil {
 		return fmt.Errorf("could not create MR discussion thread for TFC run status updates. %w", err)

pkg/tfc_trigger/tfc_trigger.go

@@ -108,7 +108,7 @@ func TestTFCEvents_SingleWorkspacePlan(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 	testSuite := mocks.CreateTestSuite(mockCtrl, mocks.TestOverrides{ProjectConfig: ws}, t)
-	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC plan for Workspace: `zapier-test/service-tfbuddy`.").Return(testSuite.MockGitDisc, nil)
+	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC plan for Workspace: `zapier-test/service-tfbuddy`.\n<!-- tfbuddy:ws=service-tfbuddy:action=plan -->").Return(testSuite.MockGitDisc, nil)
 	testSuite.MockApiClient.EXPECT().CreateRunFromSource(gomock.Any(), gomock.Any()).Return(&tfe.Run{
 		ID: "101",
 		Workspace: &tfe.Workspace{Name: "service-tfbuddy",
@@ -160,7 +160,7 @@ func TestTFCEvents_SingleWorkspacePlanError(t *testing.T) {
 	defer mockCtrl.Finish()
 	testSuite := mocks.CreateTestSuite(mockCtrl, mocks.TestOverrides{ProjectConfig: ws}, t)
 
-	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC plan for Workspace: `zapier-test/service-tfbuddy`.").Return(testSuite.MockGitDisc, nil)
+	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC plan for Workspace: `zapier-test/service-tfbuddy`.\n<!-- tfbuddy:ws=service-tfbuddy:action=plan -->").Return(testSuite.MockGitDisc, nil)
 
 	testSuite.MockApiClient.EXPECT().CreateRunFromSource(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("could not create run from source"))
 
@@ -279,8 +279,8 @@ func TestTFCEvents_MultiWorkspaceApply(t *testing.T) {
 	testSuite := mocks.CreateTestSuite(mockCtrl, mocks.TestOverrides{ProjectConfig: ws}, t)
 	testSuite.MockGitClient.EXPECT().GetMergeRequestModifiedFiles(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS).Return([]string{"main.tf", "staging/terraform.tf"}, nil)
 
-	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC apply for Workspace: `zapier-test/service-tfbuddy`.").Return(testSuite.MockGitDisc, nil)
-	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC apply for Workspace: `zapier-test/service-tfbuddy-staging`.").Return(testSuite.MockGitDisc, nil)
+	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC apply for Workspace: `zapier-test/service-tfbuddy`.\n<!-- tfbuddy:ws=service-tfbuddy:action=apply -->").Return(testSuite.MockGitDisc, nil)
+	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC apply for Workspace: `zapier-test/service-tfbuddy-staging`.\n<!-- tfbuddy:ws=service-tfbuddy-staging:action=apply -->").Return(testSuite.MockGitDisc, nil)
 
 	testSuite.MockApiClient.EXPECT().GetWorkspaceByName(gomock.Any(), "zapier-test", gomock.Any()).DoAndReturn(func(a interface{}, c, d string) (*tfe.Workspace, error) {
 		return &tfe.Workspace{ID: c}, nil
@@ -409,8 +409,8 @@ func TestTFCEvents_MultiWorkspaceApplyError(t *testing.T) {
 	testSuite := mocks.CreateTestSuite(mockCtrl, mocks.TestOverrides{ProjectConfig: ws}, t)
 	testSuite.MockGitClient.EXPECT().GetMergeRequestModifiedFiles(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS).Return([]string{"main.tf", "staging/terraform.tf"}, nil)
 
-	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC apply for Workspace: `zapier-test/service-tfbuddy`.").Return(testSuite.MockGitDisc, nil)
-	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC apply for Workspace: `zapier-test/service-tfbuddy-staging`.").Return(testSuite.MockGitDisc, nil)
+	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC apply for Workspace: `zapier-test/service-tfbuddy`.\n<!-- tfbuddy:ws=service-tfbuddy:action=apply -->").Return(testSuite.MockGitDisc, nil)
+	testSuite.MockGitClient.EXPECT().CreateMergeRequestDiscussion(gomock.Any(), testSuite.MetaData.MRIID, testSuite.MetaData.ProjectNameNS, "Starting TFC apply for Workspace: `zapier-test/service-tfbuddy-staging`.\n<!-- tfbuddy:ws=service-tfbuddy-staging:action=apply -->").Return(testSuite.MockGitDisc, nil)
 
 	testSuite.MockApiClient.EXPECT().GetWorkspaceByName(gomock.Any(), "zapier-test", gomock.Any()).DoAndReturn(func(a interface{}, b, c string) (*tfe.Workspace, error) {
 		return &tfe.Workspace{ID: c}, nil