Eng 3123 cli action comment wait (#4245)

## Summary

- Add `--comment` flag to CLI (`submit-plan`, `start-analysis`) and
`--wait` flag to `get-change` to enable GitHub App PR commenting and
control analysis polling
- Add `comment` and `wait` inputs to the `submit-plan` composite action,
with backward-compatible `fetch-change` deprecation
- Migrate internal workflows (`terraform.yml`, `terraform-meta.yml`) to
use the new inputs and disable legacy Slack plan notifications

## Linear Ticket

- **Ticket**:
[ENG-3123](https://linear.app/overmind/issue/ENG-3123/phase-4-cli-action-wire-comment-flag-auto-detect-and-skip-wait)
— Phase 4: CLI + Action — Wire Comment Flag, Auto-Detect, and Skip Wait
- **Project**: Multi-Plan Submission & GitHub App PR Commenting (Phase 4
of 5)

## Changes

### CLI (`cli/cmd/`)
- `flags.go`: New `--comment` bool flag on `addAnalysisFlags`,
requesting the GitHub App to post PR comments
- `changes_submit_plan.go`: When `--comment` is set, outputs eval-able
`CHANGE_URL` and `GITHUB_APP_ACTIVE` assignments instead of bare URL;
passes `PostGithubComment` to `StartChangeAnalysis` RPC
- `changes_start_analysis.go`: Same `--comment` behavior and
`PostGithubComment` plumbing for the standalone `start-analysis` command
- `changes_get_change.go`: Adds `--wait` flag (default `true`); skips
`waitForChangeAnalysis` when `--wait=false`. Also fixes `MarkDeprecated`
referencing wrong command (`submitPlanCmd` → `getChangeCmd`)
- Housekeeping: replace `_ = MarkDeprecated`/`MarkHidden` with
`cobra.CheckErr(...)` across 7 call sites

### Action (`actions/submit-plan/action.yml`)
- New `comment` (default `"true"`) and `wait` (default `"false"`) inputs
- `fetch-change` marked deprecated with backward-compatible shim
- New `github-app-active` output; fixes `message` output (was
incorrectly mapped to `change-url`)
- When `comment=true`: tries `--comment` flag, falls back gracefully if
CLI is older (`unknown flag` detection), and only fetches/posts sticky
comment when the GitHub App is not active
- Stderr isolation: redirects stderr to temp files (`submit-stderr.log`,
`get-stderr.log`) instead of `2>&1` to prevent logrus output from
polluting eval'd shell assignments or PR comment content

### Workflows (`.github/workflows/`)
- `terraform.yml` and `terraform-meta.yml`: migrate from `fetch-change`
to `comment`, add push-event guard, disable Slack plan notifications
(GitHub App replaces them)

## Deviations from Approved Plan

### Additions not in the plan

1. **Stderr isolation in action shell logic**
(`actions/submit-plan/action.yml`): The plan uses `eval "$(cli ...)"`
directly. The implementation captures stdout to a variable with stderr
redirected to temp files (`2>./overmindtech/submit-stderr.log`,
`2>./overmindtech/get-stderr.log`), then evals the variable. This
prevents logrus stderr lines (containing invalid bash identifiers like
`change-url`) from breaking `eval`, and prevents log noise from leaking
into PR comment content.

2. **Backward compatibility fallback for older CLIs**
(`actions/submit-plan/action.yml`): The plan assumes the CLI supports
`--comment` and `--wait`. The implementation adds fallback: if the CLI
returns "unknown flag" (detected via the stderr temp file), it falls
back to the legacy code path and logs a `::notice::`. This enables
rolling out the action change before all CLI versions support the new
flags.

3. **Push-event guard in workflows** (`.github/workflows/terraform.yml`,
`terraform-meta.yml`): The plan removes `fetch-change` without adding an
equivalent guard. The implementation passes `comment: ${{
github.event.number != '' }}` instead of unconditional `comment: true`,
preventing comment logic from running on push events where there's no PR
number.

### Minor approach changes

4. **Sticky comment condition**: The plan checks `inputs.comment !=
'false'`. The implementation checks `steps.submit-plan.outputs.message
!= ''` — more robust since `message` is only populated when the change
was actually fetched.

5. **`fetch-change` deprecation mapping**: The plan maps both `true` →
`comment: true` and `false` → `comment: false`. The implementation only
remaps the `false` case (setting `OVM_COMMENT='false'`), since `comment`
already defaults to `"true"`.

### Omissions from the plan

6. **Part 8 — Linear issue for Slack notification feature**: The plan
calls for creating a Linear issue titled "Investigate Slack notification
feature for change analysis results". This was **not created** and
should be filed separately.

## Test Plan

- [x] Flag registration tests for `--comment` on `submit-plan` and
`start-analysis`
- [x] Flag registration test for `--wait` on `get-change` (default
`true`)
- [ ] Verify `submit-plan` action with `comment: true` on a PR event
(GitHub App active path)
- [ ] Verify `submit-plan` action with `comment: true` when GitHub App
is not installed (sticky comment fallback)
- [ ] Verify `submit-plan` action with older CLI that doesn't support
`--comment` (graceful fallback)
- [ ] Verify `comment: false` skips all PR commenting logic
- [ ] Verify `wait: true` blocks until analysis completes and populates
`message` output

<!-- CURSOR_SUMMARY -->
> [!NOTE]
> **Medium Risk**
> Changes GitHub Actions and CLI behavior around when to wait for
Overmind analysis and where results are posted; misconfiguration could
lead to missing plan feedback or altered CI timing. Touches deployment
workflows but not Terraform execution logic itself.
>
> **Overview**
> Routes Terraform plan reporting away from Slack and toward PR
comments, wiring workflows to pass a new `comment` input to
`actions/submit-plan` (and disabling the plan-to-Slack steps).
>
> Updates the `submit-plan` composite action and Overmind CLI to support
`comment`/`wait` controls (deprecating `fetch-change`), including
conditional fetching of analysis results, GitHub App vs sticky-comment
handling, and new CLI flags/outputs (`--comment`, `get-change --wait`).
Also tightens flag handling by checking errors when hiding/deprecating
Cobra flags and adds targeted tests for the new flags.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
1690417e01470b73fe9bd371b2eb1f878895d32d. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

GitOrigin-RevId: a0fb20e395e0f0ffb62e32a3970c48dff980e184

Author: DavidS-ovm

cmd/changes_get_change.go

@@ -71,8 +71,10 @@ func GetChange(cmd *cobra.Command, args []string) error {
 	}
 
 	client := AuthenticatedChangesClient(ctx, oi)
-	if err := waitForChangeAnalysis(ctx, client, changeUuid, lf); err != nil {
-		return err
+	if viper.GetBool("wait") {
+		if err := waitForChangeAnalysis(ctx, client, changeUuid, lf); err != nil {
+			return err
+		}
 	}
 
 	app, _ = strings.CutSuffix(app, "/")
@@ -142,7 +144,8 @@ func init() {
 	getChangeCmd.PersistentFlags().String("status", "CHANGE_STATUS_DEFINING", "The expected status of the change. Use this with --ticket-link to get the first change with that status for a given ticket link. Allowed values: CHANGE_STATUS_DEFINING (ready for analysis/analysis in progress), CHANGE_STATUS_HAPPENING (deployment in progress), CHANGE_STATUS_DONE (deployment completed)")
 
 	getChangeCmd.PersistentFlags().String("frontend", "", "The frontend base URL")
-	_ = submitPlanCmd.PersistentFlags().MarkDeprecated("frontend", "This flag is no longer used and will be removed in a future release. Use the '--app' flag instead.") // MarkDeprecated only errors if the flag doesn't exist, we fall back to using app
+	cobra.CheckErr(getChangeCmd.PersistentFlags().MarkDeprecated("frontend", "This flag is no longer used and will be removed in a future release. Use the '--app' flag instead."))
+	getChangeCmd.PersistentFlags().Bool("wait", true, "Wait for analysis to complete before returning. Set to false to return immediately with the current status.")
 	getChangeCmd.PersistentFlags().String("format", "json", "How to render the change. Possible values: json, markdown")
 	getChangeCmd.PersistentFlags().StringSlice("risk-levels", []string{"high", "medium", "low"}, "Only show changes with the specified risk levels. Allowed values: high, medium, low")
 }

cmd/changes_get_change_test.go

@@ -7,6 +7,20 @@ import (
 	"github.com/overmindtech/cli/go/sdp-go"
 )
 
+func TestGetChangeCmdHasWaitFlag(t *testing.T) {
+	t.Parallel()
+
+	flag := getChangeCmd.PersistentFlags().Lookup("wait")
+	if flag == nil {
+		t.Error("Expected wait flag to be registered on get-change command")
+		return
+	}
+
+	if flag.DefValue != "true" {
+		t.Errorf("Expected wait flag default value to be 'true', got %q", flag.DefValue)
+	}
+}
+
 func TestValidateChangeStatus(t *testing.T) {
 	tests := []struct {
 		name        string

cmd/changes_get_signals.go

@@ -98,6 +98,6 @@ func init() {
 	getSignalsCmd.PersistentFlags().String("status", "CHANGE_STATUS_DEFINING", "The expected status of the change. Use this with --ticket-link to get the first change with that status for a given ticket link. Allowed values: CHANGE_STATUS_DEFINING (ready for analysis/analysis in progress), CHANGE_STATUS_HAPPENING (deployment in progress), CHANGE_STATUS_DONE (deployment completed)")
 
 	getSignalsCmd.PersistentFlags().String("frontend", "", "The frontend base URL")
-	_ = getSignalsCmd.PersistentFlags().MarkDeprecated("frontend", "This flag is no longer used and will be removed in a future release. Use the '--app' flag instead.") // MarkDeprecated only errors if the flag doesn't exist, we fall back to using app
+	cobra.CheckErr(getSignalsCmd.PersistentFlags().MarkDeprecated("frontend", "This flag is no longer used and will be removed in a future release. Use the '--app' flag instead."))
 	getSignalsCmd.PersistentFlags().String("format", "json", "How to render the signals. Possible values: json, markdown")
 }

cmd/changes_start_analysis.go

@@ -57,14 +57,15 @@ func StartAnalysis(cmd *cobra.Command, args []string) error {
 
 	client := AuthenticatedChangesClient(ctx, oi)
 
-	_, err = client.StartChangeAnalysis(ctx, &connect.Request[sdp.StartChangeAnalysisRequest]{
+	resp, err := client.StartChangeAnalysis(ctx, &connect.Request[sdp.StartChangeAnalysisRequest]{
 		Msg: &sdp.StartChangeAnalysisRequest{
 			ChangeUUID:                        changeUUID[:],
 			ChangingItems:                     nil, // uses pre-stored items from AddPlannedChanges
 			BlastRadiusConfigOverride:         analysisConfig.BlastRadiusConfig,
 			RoutineChangesConfigOverride:      analysisConfig.RoutineChangesConfig,
 			GithubOrganisationProfileOverride: analysisConfig.GithubOrgProfile,
 			Knowledge:                         analysisConfig.KnowledgeFiles,
+			PostGithubComment:                 viper.GetBool("comment"),
 		},
 	})
 	if err != nil {
@@ -78,7 +79,13 @@ func StartAnalysis(cmd *cobra.Command, args []string) error {
 	app, _ = strings.CutSuffix(app, "/")
 	changeUrl := fmt.Sprintf("%v/changes/%v?utm_source=cli&cli_version=%v", app, changeUUID, tracing.Version())
 	log.WithContext(ctx).WithFields(lf).WithField("change-url", changeUrl).Info("Change analysis started")
-	fmt.Println(changeUrl)
+
+	if viper.GetBool("comment") {
+		fmt.Printf("CHANGE_URL='%s'\n", changeUrl)
+		fmt.Printf("GITHUB_APP_ACTIVE='%v'\n", resp.Msg.GetGithubAppActive())
+	} else {
+		fmt.Println(changeUrl)
+	}
 
 	if viper.GetBool("wait") {
 		log.WithContext(ctx).WithFields(lf).Info("Waiting for analysis to complete")

cmd/changes_start_analysis_test.go

@@ -27,6 +27,7 @@ func TestAddAnalysisFlags(t *testing.T) {
 		{"blast-radius-max-time", "blast-radius-max-time", "duration"},
 		{"change-analysis-target-duration", "change-analysis-target-duration", "duration"},
 		{"signal-config", "signal-config", "string"},
+		{"comment", "comment", "bool"},
 	}
 
 	for _, tt := range tests {
@@ -189,6 +190,7 @@ func TestStartAnalysisCmdFlags(t *testing.T) {
 		{"blast-radius-link-depth", "blast-radius-link-depth"},
 		{"blast-radius-max-items", "blast-radius-max-items"},
 		{"signal-config", "signal-config"},
+		{"comment flag", "comment"},
 	}
 
 	for _, tt := range tests {
@@ -205,6 +207,20 @@ func TestStartAnalysisCmdFlags(t *testing.T) {
 	}
 }
 
+func TestSubmitPlanCmdHasCommentFlag(t *testing.T) {
+	t.Parallel()
+
+	flag := submitPlanCmd.PersistentFlags().Lookup("comment")
+	if flag == nil {
+		t.Error("Expected comment flag to be registered on submit-plan command")
+		return
+	}
+
+	if flag.DefValue != "false" {
+		t.Errorf("Expected comment flag default value to be 'false', got %q", flag.DefValue)
+	}
+}
+
 func TestSubmitPlanCmdHasNoStartFlag(t *testing.T) {
 	t.Parallel()
 

cmd/changes_submit_plan.go

@@ -258,7 +258,12 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
 		log.WithContext(ctx).WithFields(lf).Info("Re-using change")
 	}
 
+	var githubAppActive bool
+
 	if viper.GetBool("no-start") {
+		if viper.GetBool("comment") {
+			log.WithContext(ctx).WithFields(lf).Info("--comment has no effect with --no-start; pass --comment to start-analysis instead")
+		}
 		// Store planned changes without starting analysis (multi-plan workflow)
 		_, err = client.AddPlannedChanges(ctx, &connect.Request[sdp.AddPlannedChangesRequest]{
 			Msg: &sdp.AddPlannedChangesRequest{
@@ -281,14 +286,15 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
 			return err
 		}
 
-		_, err = client.StartChangeAnalysis(ctx, &connect.Request[sdp.StartChangeAnalysisRequest]{
+		resp, err := client.StartChangeAnalysis(ctx, &connect.Request[sdp.StartChangeAnalysisRequest]{
 			Msg: &sdp.StartChangeAnalysisRequest{
 				ChangeUUID:                        changeUUID[:],
 				ChangingItems:                     plannedChanges,
 				BlastRadiusConfigOverride:         analysisConfig.BlastRadiusConfig,
 				RoutineChangesConfigOverride:      analysisConfig.RoutineChangesConfig,
 				GithubOrganisationProfileOverride: analysisConfig.GithubOrgProfile,
 				Knowledge:                         analysisConfig.KnowledgeFiles,
+				PostGithubComment:                 viper.GetBool("comment"),
 			},
 		})
 		if err != nil {
@@ -298,12 +304,19 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
 				message: "Failed to start change analysis",
 			}
 		}
+		githubAppActive = resp.Msg.GetGithubAppActive()
 	}
 
 	app, _ = strings.CutSuffix(app, "/")
 	changeUrl := fmt.Sprintf("%v/changes/%v?utm_source=cli&cli_version=%v", app, changeUUID, tracing.Version())
 	log.WithContext(ctx).WithFields(lf).WithField("change-url", changeUrl).Info("Change ready")
-	fmt.Println(changeUrl)
+
+	if viper.GetBool("comment") {
+		fmt.Printf("CHANGE_URL='%s'\n", changeUrl)
+		fmt.Printf("GITHUB_APP_ACTIVE='%v'\n", githubAppActive)
+	} else {
+		fmt.Println(changeUrl)
+	}
 
 	return nil
 }
@@ -385,7 +398,7 @@ func init() {
 	addAnalysisFlags(submitPlanCmd)
 
 	submitPlanCmd.PersistentFlags().String("frontend", "", "The frontend base URL")
-	_ = submitPlanCmd.PersistentFlags().MarkDeprecated("frontend", "This flag is no longer used and will be removed in a future release. Use the '--app' flag instead.") // MarkDeprecated only errors if the flag doesn't exist, we fall back to using app
+	cobra.CheckErr(submitPlanCmd.PersistentFlags().MarkDeprecated("frontend", "This flag is no longer used and will be removed in a future release. Use the '--app' flag instead."))
 
 	submitPlanCmd.PersistentFlags().String("auto-tag-rules", "", "The path to the auto-tag rules file. If not provided, it will check the default location which is '.overmind/auto-tag-rules.yaml'. If no rules are found locally, the rules configured through the UI are used.")
 

cmd/explore.go

@@ -666,7 +666,7 @@ func init() {
 
 	// hidden flag to enable Azure preview support
 	exploreCmd.PersistentFlags().Bool("enable-azure-preview", false, "Enable Azure source support (preview feature).")
-	exploreCmd.PersistentFlags().MarkHidden("enable-azure-preview") //nolint:errcheck // not possible to error
+	cobra.CheckErr(exploreCmd.PersistentFlags().MarkHidden("enable-azure-preview"))
 }
 
 // unifiedGCPConfigs collates the given GCP configs by project ID.

cmd/flags.go

@@ -130,9 +130,10 @@ func addAnalysisFlags(cmd *cobra.Command) {
 	cmd.PersistentFlags().Int32("blast-radius-link-depth", 0, "Used in combination with '--blast-radius-max-items' to customise how many levels are traversed when calculating the blast radius. Larger numbers will result in a more comprehensive blast radius, but may take longer to calculate. Defaults to the account level settings.")
 	cmd.PersistentFlags().Int32("blast-radius-max-items", 0, "Used in combination with '--blast-radius-link-depth' to customise how many items are included in the blast radius. Larger numbers will result in a more comprehensive blast radius, but may take longer to calculate. Defaults to the account level settings.")
 	cmd.PersistentFlags().Duration("blast-radius-max-time", 0, "Maximum time duration for blast radius calculation (e.g., '5m', '15m', '30m'). When the time limit is reached, the analysis continues with risks identified up to that point. Defaults to the account level settings (QUICK: 10m, DETAILED: 15m, FULL: 30m). Valid range: 1m to 30m.")
-	_ = cmd.PersistentFlags().MarkDeprecated("blast-radius-max-time", "This flag is no longer used and will be removed in a future release. Use the '--change-analysis-target-duration' flag instead.")
+	cobra.CheckErr(cmd.PersistentFlags().MarkDeprecated("blast-radius-max-time", "This flag is no longer used and will be removed in a future release. Use the '--change-analysis-target-duration' flag instead."))
 	cmd.PersistentFlags().Duration("change-analysis-target-duration", 0, "Target duration for change analysis planning (e.g., '5m', '15m', '30m'). This is NOT a hard deadline - the blast radius phase uses 67% of this target to stop gracefully. The job can run slightly past this target and is only hard-stopped at 30 minutes. Defaults to the account level settings (QUICK: 10m, DETAILED: 15m, FULL: 30m). Valid range: 1m to 30m.")
 	cmd.PersistentFlags().String("signal-config", "", "The path to the signal config file. If not provided, it will check the default location which is '.overmind/signal-config.yaml'. If no config is found locally, the config configured through the UI is used.")
+	cmd.PersistentFlags().Bool("comment", false, "Request the GitHub App to post analysis results as a PR comment. Requires the account to have the Overmind GitHub App installed with pull_requests:write.")
 }
 
 // AnalysisConfig holds all the configuration needed to start change analysis.

cmd/knowledge_list.go

@@ -107,5 +107,5 @@ func init() {
 	knowledgeCmd.AddCommand(knowledgeListCmd)
 
 	knowledgeListCmd.Flags().String("dir", ".", "Directory to start searching from")
-	knowledgeListCmd.Flags().MarkHidden("dir") //nolint:errcheck // not possible to error
+	cobra.CheckErr(knowledgeListCmd.Flags().MarkHidden("dir"))
 }

cmd/terraform.go

@@ -35,7 +35,7 @@ func init() {
 
 	// hidden flag to enable Azure preview support
 	terraformCmd.PersistentFlags().Bool("enable-azure-preview", false, "Enable Azure source support (preview feature).")
-	terraformCmd.PersistentFlags().MarkHidden("enable-azure-preview") //nolint:errcheck // not possible to error
+	cobra.CheckErr(terraformCmd.PersistentFlags().MarkHidden("enable-azure-preview"))
 }
 
 var applyOnlyArgs = []string{