diff --git a/internal/actions/submit/events.go b/internal/actions/submit/events.go index e705d17b2..ce6aa0e84 100644 --- a/internal/actions/submit/events.go +++ b/internal/actions/submit/events.go @@ -44,13 +44,27 @@ func (PreparingEvent) submitEvent() {} type BranchPlanEvent struct { BranchName string Action string // "create" or "update" + PRNumber *int // existing PR number for updates, nil for creates IsCurrent bool + Empty bool // branch has no commits relative to its parent Skipped bool SkipReason string } func (BranchPlanEvent) submitEvent() {} +// BranchWarningEvent surfaces a non-fatal warning raised while submitting a +// branch (e.g. labels or reviewers could not be applied). Warnings must flow +// through the handler rather than direct console output: the interactive +// runner quiets the console while the TUI is active, so direct writes during +// the submission phase are silently dropped. +type BranchWarningEvent struct { + BranchName string + Warning string +} + +func (BranchWarningEvent) submitEvent() {} + // SubmissionStartEvent indicates the submission phase is beginning. type SubmissionStartEvent struct { Branches []BranchInfo @@ -68,10 +82,29 @@ type BranchProgressEvent struct { func (BranchProgressEvent) submitEvent() {} +// CompletionOutcome classifies how a submit run ended, so handlers can decide +// presentation without string-matching messages. +type CompletionOutcome string + +// CompletionOutcome values. +const ( + OutcomeComplete CompletionOutcome = "complete" // branches were submitted + OutcomeUpToDate CompletionOutcome = "up-to-date" // nothing needed submitting + OutcomeDryRun CompletionOutcome = "dry-run" // plan reported, nothing submitted + OutcomeCanceled CompletionOutcome = "canceled" // user declined a confirmation + OutcomeNothingToSubmit CompletionOutcome = "nothing-to-submit" // no branches in scope / untracked + OutcomeFailed CompletionOutcome = "failed" // at least one branch failed +) + // CompletionEvent indicates the action has finished. type CompletionEvent struct { - Success bool - Message string // "All PRs up to date", "Dry run complete", etc. + Outcome CompletionOutcome + Message string // human-readable detail, e.g. "All PRs up to date" +} + +// Success reports whether the run ended without failure or cancellation. +func (e CompletionEvent) Success() bool { + return e.Outcome != OutcomeFailed && e.Outcome != OutcomeCanceled } func (CompletionEvent) submitEvent() {} diff --git a/internal/actions/submit/planning.go b/internal/actions/submit/planning.go index d69b8e07b..301846269 100644 --- a/internal/actions/submit/planning.go +++ b/internal/actions/submit/planning.go @@ -11,8 +11,9 @@ import ( // prepareBranchesForSubmit prepares submission info for each branch, emitting // events via handler. When remoteStatuses is provided, planning reuses that // snapshot; otherwise it asks the engine for lazily batched status so create-only -// dry runs do not read remote state. -func prepareBranchesForSubmit(ctx *app.Context, branches engine.Branches, opts Options, currentBranch string, remoteStatuses engine.BranchRemoteStatuses, handler Handler) ([]Info, error) { +// dry runs do not read remote state. empty flags branches with no commits so +// plan output can surface them. +func prepareBranchesForSubmit(ctx *app.Context, branches engine.Branches, opts Options, currentBranch string, remoteStatuses engine.BranchRemoteStatuses, empty map[string]bool, handler Handler) ([]Info, error) { submissionInfos := make([]Info, 0, len(branches)) nav := ctx.Navigator() @@ -65,7 +66,7 @@ func prepareBranchesForSubmit(ctx *app.Context, branches engine.Branches, opts O Action: action, IsCurrent: isCurrent, Skipped: true, - SkipReason: "skipped, no existing PR", + SkipReason: "no existing PR", }) continue } @@ -88,6 +89,7 @@ func prepareBranchesForSubmit(ctx *app.Context, branches engine.Branches, opts O handler.OnEvent(BranchPlanEvent{ BranchName: branchName, Action: action, + PRNumber: prNumber, IsCurrent: isCurrent, Skipped: true, SkipReason: status.Reason, @@ -140,7 +142,9 @@ func prepareBranchesForSubmit(ctx *app.Context, branches engine.Branches, opts O handler.OnEvent(BranchPlanEvent{ BranchName: branchName, Action: action, + PRNumber: prNumber, IsCurrent: isCurrent, + Empty: empty[branchName], Skipped: false, }) @@ -159,6 +163,33 @@ func prepareBranchesForSubmit(ctx *app.Context, branches engine.Branches, opts O return submissionInfos, nil } +// confirmPrompt describes what --confirm is about to do in concrete terms. +func confirmPrompt(infos []Info) string { + creates, updates := 0, 0 + for _, info := range infos { + if info.Action == actionCreate { + creates++ + } else { + updates++ + } + } + switch { + case creates> 0 && updates> 0: + return fmt.Sprintf("Create %d and update %d PRs?", creates, updates) + case creates> 0: + return fmt.Sprintf("Create %d %s?", creates, pluralPR(creates)) + default: + return fmt.Sprintf("Update %d %s?", updates, pluralPR(updates)) + } +} + +func pluralPR(count int) string { + if count == 1 { + return "PR" + } + return "PRs" +} + // getBranchesToSubmit returns the list of branches to submit based on options func getBranchesToSubmit(ctx *app.Context, opts Options) ([]string, error) { nav := ctx.Navigator() diff --git a/internal/actions/submit/submit.go b/internal/actions/submit/submit.go index 8f0adb18c..ede98023f 100644 --- a/internal/actions/submit/submit.go +++ b/internal/actions/submit/submit.go @@ -97,7 +97,7 @@ func Action(ctx *app.Context, opts Options, handler Handler) error { // Non-interactive: inform and exit gracefully ctx.Output.Info("Branch %s is not tracked by stackit.", branchName) ctx.Output.Tip("Run 'stackit track' to track this branch, then try again.") - handler.OnEvent(CompletionEvent{Success: true, Message: "Branch not tracked"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeNothingToSubmit, Message: "Branch not tracked"}) return nil } @@ -110,7 +110,7 @@ func Action(ctx *app.Context, opts Options, handler Handler) error { } if !shouldTrack { ctx.Output.Info("Skipping. Use 'stackit track --parent ' for a different parent.") - handler.OnEvent(CompletionEvent{Success: true, Message: "Tracking declined"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeNothingToSubmit, Message: "Tracking declined"}) return nil } @@ -127,7 +127,7 @@ func Action(ctx *app.Context, opts Options, handler Handler) error { return err } if len(branches) == 0 { - handler.OnEvent(CompletionEvent{Success: true, Message: "No branches to submit"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeNothingToSubmit, Message: "No branches to submit"}) return nil } @@ -204,31 +204,58 @@ func Action(ctx *app.Context, opts Options, handler Handler) error { remoteStatuses = <-remotestatusch } + // Branches with no commits still submit (an empty PR can be a placeholder), + // but the plan flags them and interactive runs confirm first. + emptyMap := eng.BatchIsBranchEmpty(branches) + // Prepare branches for submit (show planning phase with current indicator) - submissionInfos, err := prepareBranchesForSubmit(ctx, branchObjs, opts, currentBranchName, remoteStatuses, handler) + submissionInfos, err := prepareBranchesForSubmit(ctx, branchObjs, opts, currentBranchName, remoteStatuses, emptyMap, handler) if err != nil { return fmt.Errorf("failed to prepare branches: %w", err) } // Check if we should abort if opts.DryRun { - handler.OnEvent(CompletionEvent{Success: true, Message: "Dry run complete"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeDryRun, Message: "Dry run complete"}) return nil } if len(submissionInfos) == 0 { - handler.OnEvent(CompletionEvent{Success: true, Message: "All PRs up to date"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeUpToDate, Message: "All PRs up to date"}) return nil } + // Confirm empty branches interactively. --confirm already prompts below + // with the full plan visible, so don't ask twice. + emptyCount := 0 + for _, info := range submissionInfos { + if emptyMap[info.BranchName] { + emptyCount++ + } + } + if emptyCount> 0 && handler.IsInteractive() && !opts.Confirm { + message := fmt.Sprintf("%d branches have no commits — submit anyway?", emptyCount) + if emptyCount == 1 { + message = "1 branch has no commits — submit anyway?" + } + confirmed, err := handler.Confirm(message, true) + if err != nil { + return fmt.Errorf("confirmation canceled: %w", err) + } + if !confirmed { + handler.OnEvent(CompletionEvent{Outcome: OutcomeCanceled, Message: "Submit canceled"}) + return nil + } + } + // Handle interactive confirmation if opts.Confirm { - confirmed, err := handler.Confirm("Proceed with submit?", true) + confirmed, err := handler.Confirm(confirmPrompt(submissionInfos), true) if err != nil { return fmt.Errorf("confirmation canceled: %w", err) } if !confirmed { - handler.OnEvent(CompletionEvent{Success: false, Message: "Submit canceled"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeCanceled, Message: "Submit canceled"}) return nil } } @@ -297,7 +324,7 @@ func Action(ctx *app.Context, opts Options, handler Handler) error { } if submitErr != nil { - handler.OnEvent(CompletionEvent{Success: false, Message: "Submit failed"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeFailed, Message: "Submit failed"}) return submitErr } @@ -321,13 +348,13 @@ func Action(ctx *app.Context, opts Options, handler Handler) error { // Push metadata refs for successfully submitted branches if err := pushMetadataRefs(ctx, branchObjs); err != nil { - handler.OnEvent(CompletionEvent{Success: false, Message: "Submit failed"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeFailed, Message: "Submit failed"}) return fmt.Errorf("failed to push metadata to remote: %w. Your PRs were created/updated successfully, but metadata sync failed. Run 'st sync' and try submitting again", err) } ctx.Logger.Info("submit completed branchCount=%v", len(branches)) - handler.OnEvent(CompletionEvent{Success: true, Message: "Submit complete"}) + handler.OnEvent(CompletionEvent{Outcome: OutcomeComplete, Message: "Submit complete"}) return nil } @@ -385,9 +412,9 @@ func submitBranch(ctx *app.Context, info Info, opts Options, handler Handler, re var prURL string var err error if info.Action == actionCreate { - prURL, err = createPullRequestQuiet(ctx, info, repoOwner, repoName) + prURL, err = createPullRequestQuiet(ctx, info, repoOwner, repoName, handler) } else { - prURL, err = updatePullRequestQuiet(ctx, info, opts, repoOwner, repoName) + prURL, err = updatePullRequestQuiet(ctx, info, opts, repoOwner, repoName, handler) } if err != nil { @@ -477,7 +504,7 @@ func pushSubmittedBranches(ctx *app.Context, opts Options, infos []Info, remote } // createPullRequestQuiet creates a new pull request without logging -func createPullRequestQuiet(ctx *app.Context, submissionInfo Info, repoOwner, repoName string) (string, error) { +func createPullRequestQuiet(ctx *app.Context, submissionInfo Info, repoOwner, repoName string, handler Handler) (string, error) { pr := ctx.PR() nav := ctx.Navigator() @@ -506,9 +533,9 @@ func createPullRequestQuiet(ctx *app.Context, submissionInfo Info, repoOwner, re return "", fmt.Errorf("failed to create PR for %s: %w", submissionInfo.BranchName, err) } - // Log any warnings from PR creation (e.g., failed to add labels/assignees) + // Surface any warnings from PR creation (e.g., failed to add labels/assignees) for _, warning := range prResult.Warnings { - ctx.Output.Warn("%s: %s", submissionInfo.BranchName, warning) + handler.OnEvent(BranchWarningEvent{BranchName: submissionInfo.BranchName, Warning: warning}) } // Update PR info @@ -531,7 +558,7 @@ func createPullRequestQuiet(ctx *app.Context, submissionInfo Info, repoOwner, re } // updatePullRequestQuiet updates an existing pull request without logging -func updatePullRequestQuiet(ctx *app.Context, submissionInfo Info, opts Options, repoOwner, repoName string) (string, error) { +func updatePullRequestQuiet(ctx *app.Context, submissionInfo Info, opts Options, repoOwner, repoName string, handler Handler) (string, error) { pr := ctx.PR() nav := ctx.Navigator() @@ -630,9 +657,9 @@ func updatePullRequestQuiet(ctx *app.Context, submissionInfo Info, opts Options, return "", fmt.Errorf("failed to update PR for %s: %w", submissionInfo.BranchName, err) } - // Log any warnings from PR update (e.g., failed to add labels/assignees) + // Surface any warnings from PR update (e.g., failed to add labels/assignees) for _, warning := range updateWarnings { - ctx.Output.Warn("%s: %s", submissionInfo.BranchName, warning) + handler.OnEvent(BranchWarningEvent{BranchName: submissionInfo.BranchName, Warning: warning}) } // If the base.sha refresh was needed but failed, keep the previously stored BaseSHA diff --git a/internal/actions/submit/submit_validation.go b/internal/actions/submit/submit_validation.go index 7094535f3..cc8871e40 100644 --- a/internal/actions/submit/submit_validation.go +++ b/internal/actions/submit/submit_validation.go @@ -60,11 +60,6 @@ func ValidateBranchesToSubmit(ctx *app.Context, branches []string) error { return err } - // Validate no empty branches - if err := validateNoEmptyBranches(branches, ctx); err != nil { - return err - } - // Validate no merged/closed branches if err := validateNoMergedOrClosedBranches(branches, ctx.Status(), ctx); err != nil { return err @@ -115,7 +110,7 @@ func validateBaseRevisions(branches []string, eng engine.BranchStatus, ctx *app. switch { case parentBranch.IsTrunk(): if !branch.IsBranchUpToDate() { - ctx.Output.Info("Note that %s has fallen behind trunk. You may encounter conflicts if you attempt to merge it.", + ctx.Output.Warn("%s is behind trunk and may conflict on merge — run 'stackit sync' and 'stackit restack' to update it.", output.Branch(branchName, false)) } case validatedBranches[parentBranchName]: @@ -138,36 +133,6 @@ func validateBaseRevisions(branches []string, eng engine.BranchStatus, ctx *app. return nil } -// validateNoEmptyBranches checks for empty branches and prompts user if found -func validateNoEmptyBranches(branches []string, runtimeCtx *app.Context) error { - // Resolve emptiness for the whole set in one batched rev-parse rather than a - // diff per branch. - empty := runtimeCtx.Engine.BatchIsBranchEmpty(branches) - emptyBranches := []string{} - for _, branchName := range branches { - if empty[branchName] { - emptyBranches = append(emptyBranches, branchName) - } - } - - if len(emptyBranches) == 0 { - return nil - } - - hasMultiple := len(emptyBranches)> 1 - runtimeCtx.Output.Warn("The following branch%s have no changes:", actions.PluralSuffix("branch", hasMultiple)) - for _, b := range emptyBranches { - runtimeCtx.Output.Warn("▸ %s", b) - } - runtimeCtx.Output.Warn("Are you sure you want to submit %s?", actions.PluralIt(hasMultiple)) - - // For now, we'll allow empty branches (non-interactive mode) - // In interactive mode, we would prompt here - // TODO: Add interactive prompt when needed - - return nil -} - // validateNoMergedOrClosedBranches checks for merged/closed PRs and prompts user if found func validateNoMergedOrClosedBranches(branches []string, eng engine.BranchStatus, ctx *app.Context) error { mergedOrClosedBranches := []string{} @@ -187,11 +152,11 @@ func validateNoMergedOrClosedBranches(branches []string, eng engine.BranchStatus } hasMultiple := len(mergedOrClosedBranches)> 1 - ctx.Output.Tip("You can use 'stackit sync' to find and delete all merged/closed branches automatically and rebase their children.") ctx.Output.Warn("PR%s for the following branch%s already been merged or closed:", actions.PluralSuffix("PR", hasMultiple), actions.PluralSuffix("branch", hasMultiple)) for _, b := range mergedOrClosedBranches { ctx.Output.Warn("▸ %s", b) } + ctx.Output.Tip("Run 'stackit sync' to delete merged/closed branches and rebase their children.") // For now, we'll allow creating new PRs (non-interactive mode) // In interactive mode, we would prompt here diff --git a/internal/api/handlers/submit.go b/internal/api/handlers/submit.go index 893b6ca28..817d6e1e6 100644 --- a/internal/api/handlers/submit.go +++ b/internal/api/handlers/submit.go @@ -109,7 +109,7 @@ func (h *SubmitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } branches = append(branches, br) case submit.CompletionEvent: - success = e.Success + success = e.Success() completionMsg = e.Message } } diff --git a/internal/cli/stack/submit.go b/internal/cli/stack/submit.go index 9c46fdda6..dd626d1cb 100644 --- a/internal/cli/stack/submit.go +++ b/internal/cli/stack/submit.go @@ -133,7 +133,7 @@ func executeSubmit(cmd *cobra.Command, f *submitFlags) error { } // Action is the single source of truth for what to submit. The runner - // starts lazily (on the first stack-display event), so calling Action + // starts lazily (when the submission phase begins), so calling Action // unconditionally no longer flashes the TUI when there's nothing to do. runner, handler := NewSubmitUI(ctx.Output, ctx.Logger) defer runner.Cleanup() diff --git a/internal/cli/stack/submit_handlers.go b/internal/cli/stack/submit_handlers.go index 838102bcd..ce02dec98 100644 --- a/internal/cli/stack/submit_handlers.go +++ b/internal/cli/stack/submit_handlers.go @@ -1,6 +1,8 @@ package stack import ( + "fmt" + "github.com/getstackit/stackit/internal/actions/submit" "github.com/getstackit/stackit/internal/cli/common" "github.com/getstackit/stackit/internal/output" @@ -62,9 +64,18 @@ func (p *planPrinter) PrintLine(ev submit.BranchPlanEvent) { if ev.Skipped { p.out.Info("%s%s %s", marker, style.ColorDim(name), style.ColorDim("— "+ev.SkipReason)) - } else { - p.out.Info("%s%s → %s", marker, name, ev.Action) + return + } + + action := ev.Action + if ev.PRNumber != nil { + action = fmt.Sprintf("%s #%d", action, *ev.PRNumber) + } + line := fmt.Sprintf("%s%s → %s", marker, name, action) + if ev.Empty { + line += " " + style.ColorDim("(empty)") } + p.out.Info("%s", line) } // SimpleSubmitHandler implements submit.Handler with line-by-line output @@ -145,11 +156,8 @@ func (h *SimpleSubmitHandler) OnEvent(e submit.Event) { switch ev.Status { case submit.StatusSubmitting: - action := "Creating" - if item.action == "update" { - action = "Updating" - } - h.Output.Info(" ⋯ %s %s...", submitComponent.DisplayBranchName(ev.BranchName), action) + // Quiet: line-by-line output has no animation, so the transitional + // state adds noise without information; only terminal states print. case submit.StatusSyncing: // Quiet: the metadata footer sync carries no new information; the @@ -165,30 +173,35 @@ func (h *SimpleSubmitHandler) OnEvent(e submit.Event) { if item.action == "update" { actionDone = "updated" } - ref := submitComponent.PRRef(item.toSubmitItem()) - if ref != "" { - h.Output.Info(" ✓ %s %s", submitComponent.DisplayBranchName(ev.BranchName), ref) - } else { - h.Output.Info(" ✓ %s %s", submitComponent.DisplayBranchName(ev.BranchName), actionDone) + detail := actionDone + if ref := submitComponent.PRRef(item.toSubmitItem()); ref != "" { + detail = ref + " " + actionDone } + h.Output.Info(" ✓ %s %s", submitComponent.DisplayBranchName(ev.BranchName), detail) case submit.StatusError: h.Output.Info(" ✗ %s failed: %v", submitComponent.DisplayBranchName(ev.BranchName), ev.Error) } + case submit.BranchWarningEvent: + h.Output.Warn("%s: %s", submitComponent.DisplayBranchName(ev.BranchName), ev.Warning) + case submit.CompletionEvent: - switch { - case !ev.Success && ev.Message != "": - h.Output.Info("%s", ev.Message) - case ev.Success && ev.Message == "Submit complete": + switch ev.Outcome { + case submit.OutcomeComplete: if summary := submitComponent.FormatURLSummary(h.submitItems()); summary != "" { h.Output.Newline() h.Output.Info("%s", summary) } - case ev.Success && ev.Message != "": - // Dry run, all up to date, nothing to submit: print the outcome so - // the run doesn't end silently. - h.Output.Info("%s", ev.Message) + case submit.OutcomeFailed: + // Quiet: the returned error is reported by the CLI; printing the + // generic message here would double-report the failure. + default: + // Dry run, all up to date, canceled, nothing to submit: print the + // outcome so the run doesn't end silently. + if ev.Message != "" { + h.Output.Info("%s", ev.Message) + } } } } @@ -284,7 +297,6 @@ func (h *InteractiveSubmitHandler) OnEvent(e submit.Event) { // Start the TUI now that there's real submission work to animate. // Idempotent, so later events that arrive after this are safe. h.runner.Start() - h.runner.Send(submitComponent.GlobalMessageMsg("Submitting...")) case submit.BranchProgressEvent: if !h.inSubmitPhase { @@ -298,21 +310,27 @@ func (h *InteractiveSubmitHandler) OnEvent(e submit.Event) { Err: ev.Error, }) + case submit.BranchWarningEvent: + // The runner quiets console output while the TUI runs, so warnings must + // flow through the model to be rendered and persisted. + if h.runner.IsRunning() { + h.runner.Send(submitComponent.WarningMsg{ + BranchName: ev.BranchName, + Warning: ev.Warning, + }) + return + } + h.out.Warn("%s: %s", submitComponent.DisplayBranchName(ev.BranchName), ev.Warning) + case submit.CompletionEvent: - // If no stack was ever displayed (e.g. nothing to submit), the TUI was - // never started — print the outcome plainly instead of routing it - // through a runner that isn't running. + // If the submission phase never started (nothing to submit, dry run, + // canceled), the TUI isn't running — print the outcome plainly. if !h.runner.IsRunning() { - if ev.Message != "" { + if ev.Outcome != submit.OutcomeFailed && ev.Message != "" { h.out.Info("%s", ev.Message) } return } - if ev.Message != "" && ev.Message != "Submit complete" { - h.runner.Send(submitComponent.GlobalMessageMsg(ev.Message)) - } else { - h.runner.Send(submitComponent.GlobalMessageMsg("")) - } h.runner.Send(submitComponent.ProgressCompleteMsg{}) h.runner.Wait() } diff --git a/internal/cli/stack/submit_handlers_test.go b/internal/cli/stack/submit_handlers_test.go index 3747b1f42..fe0ad5509 100644 --- a/internal/cli/stack/submit_handlers_test.go +++ b/internal/cli/stack/submit_handlers_test.go @@ -29,11 +29,11 @@ func TestSimpleSubmitHandlerPrintsFinalURLSummary(t *testing.T) { Status: submitAction.StatusDone, URL: "https://github.com/getstackit/stackit/pull/934", }) - handler.OnEvent(submitAction.CompletionEvent{Success: true, Message: "Submit complete"}) + handler.OnEvent(submitAction.CompletionEvent{Outcome: submitAction.OutcomeComplete, Message: "Submit complete"}) got := out.String() require.Contains(t, got, "Submitting 1 branch") - require.Contains(t, got, "✓ guard-runner.repoRoot-reads-with-repoMu-to-fix #934") + require.Contains(t, got, "✓ guard-runner.repoRoot-reads-with-repoMu-to-fix #934 updated") require.Contains(t, got, "Pull requests") require.Contains(t, got, "#934 guard-runner.repoRoot-reads-with-repoMu-to-fix") require.Contains(t, got, " https://github.com/getstackit/stackit/pull/934") @@ -66,7 +66,7 @@ func TestSimpleSubmitHandlerPreservesURLAcrossFooterSync(t *testing.T) { BranchName: branch, Status: submitAction.StatusDone, }) - handler.OnEvent(submitAction.CompletionEvent{Success: true, Message: "Submit complete"}) + handler.OnEvent(submitAction.CompletionEvent{Outcome: submitAction.OutcomeComplete, Message: "Submit complete"}) got := out.String() require.Contains(t, got, "https://github.com/getstackit/stackit/pull/934") @@ -127,7 +127,7 @@ func TestInteractiveSubmitHandlerPrintsPlanWithoutStartingTUI(t *testing.T) { SkipReason: "no changes", IsCurrent: true, }) - handler.OnEvent(submitAction.CompletionEvent{Success: true, Message: "All PRs up to date"}) + handler.OnEvent(submitAction.CompletionEvent{Outcome: submitAction.OutcomeUpToDate, Message: "All PRs up to date"}) got := out.String() require.Contains(t, got, "Stack to submit:") @@ -136,6 +136,60 @@ func TestInteractiveSubmitHandlerPrintsPlanWithoutStartingTUI(t *testing.T) { require.Contains(t, got, "All PRs up to date") } +func TestPlanPrinterShowsPRNumbersAndEmptyAnnotation(t *testing.T) { + t.Parallel() + + out := output.NewTestOutput() + handler := NewSimpleSubmitHandler(out) + prNumber := 1189 + + handler.OnEvent(submitAction.StackDisplayEvent{Stack: submitAction.StackSnapshot{ + Branches: []string{"update-me", "empty-one"}, + TrunkBranch: "main", + }}) + handler.OnEvent(submitAction.BranchPlanEvent{ + BranchName: "update-me", + Action: "update", + PRNumber: &prNumber, + }) + handler.OnEvent(submitAction.BranchPlanEvent{ + BranchName: "empty-one", + Action: "create", + Empty: true, + }) + + got := out.String() + require.Contains(t, got, "update-me → update #1189") + require.Contains(t, got, "empty-one → create") + require.Contains(t, got, "(empty)") +} + +func TestSimpleSubmitHandlerPrintsBranchWarnings(t *testing.T) { + t.Parallel() + + out := output.NewTestOutput() + handler := NewSimpleSubmitHandler(out) + + handler.OnEvent(submitAction.BranchWarningEvent{ + BranchName: "jonnii/20260511011552/add-feature", + Warning: "failed to add labels: not found", + }) + + require.Contains(t, out.String(), "add-feature: failed to add labels: not found") +} + +func TestSimpleSubmitHandlerStaysQuietOnFailureOutcome(t *testing.T) { + t.Parallel() + + out := output.NewTestOutput() + handler := NewSimpleSubmitHandler(out) + + // The CLI prints the returned error; the handler must not double-report. + handler.OnEvent(submitAction.CompletionEvent{Outcome: submitAction.OutcomeFailed, Message: "Submit failed"}) + + require.NotContains(t, out.String(), "Submit failed") +} + func TestSimpleSubmitHandlerPrintsOutcomeWhenNothingSubmitted(t *testing.T) { t.Parallel() @@ -153,7 +207,7 @@ func TestSimpleSubmitHandlerPrintsOutcomeWhenNothingSubmitted(t *testing.T) { Skipped: true, SkipReason: "no changes", }) - handler.OnEvent(submitAction.CompletionEvent{Success: true, Message: "All PRs up to date"}) + handler.OnEvent(submitAction.CompletionEvent{Outcome: submitAction.OutcomeUpToDate, Message: "All PRs up to date"}) require.Contains(t, out.String(), "All PRs up to date") } diff --git a/internal/cli/stack/submit_test.go b/internal/cli/stack/submit_test.go index 817ebf89d..e0836a961 100644 --- a/internal/cli/stack/submit_test.go +++ b/internal/cli/stack/submit_test.go @@ -34,13 +34,9 @@ func TestSubmitCommand(t *testing.T) { // 1. Basic submit (downstack) output := runCliCommandSuccess(t, binaryPath, scene.Dir, "submit", "--dry-run", "--no-edit", "--draft", "--no-interactive") expected := testhelpers.NormalizeOutput(` -⚠️ The following branches have no changes: -⚠️ ▸ branch-a -⚠️ ▸ branch-b -⚠️ Are you sure you want to submit them? Stack to submit: - branch-a → create -●くろまる branch-b → create + branch-a → create (empty) +●くろまる branch-b → create (empty) Dry run complete `) require.Equal(t, expected, testhelpers.NormalizeOutput(output)) @@ -48,15 +44,10 @@ Dry run complete // 2. Submit --stack (full stack) output = runCliCommandSuccess(t, binaryPath, scene.Dir, "submit", "--stack", "--dry-run", "--no-edit", "--draft", "--no-interactive") expectedStack := testhelpers.NormalizeOutput(` -⚠️ The following branches have no changes: -⚠️ ▸ branch-a -⚠️ ▸ branch-b -⚠️ ▸ branch-c -⚠️ Are you sure you want to submit them? Stack to submit: - branch-a → create -●くろまる branch-b → create - branch-c → create + branch-a → create (empty) +●くろまる branch-b → create (empty) + branch-c → create (empty) Dry run complete `) require.Equal(t, expectedStack, testhelpers.NormalizeOutput(output)) diff --git a/internal/operations/submit.go b/internal/operations/submit.go index 4d2d44d62..c27d8a140 100644 --- a/internal/operations/submit.go +++ b/internal/operations/submit.go @@ -201,7 +201,7 @@ func (o *SubmitOperation) OnEvent(e submitAction.Event) { case submitAction.CompletionEvent: status := StatusCompleted - if !ev.Success { + if !ev.Success() { status = StatusFailed } o.emit(Progress{ diff --git a/internal/tui/components/submit/format.go b/internal/tui/components/submit/format.go index 5da792006..6e9d44e89 100644 --- a/internal/tui/components/submit/format.go +++ b/internal/tui/components/submit/format.go @@ -120,11 +120,11 @@ func rowParts(item Item, spinnerView string, styles Styles) (string, string) { case StatusSyncing: return spinnerView, styles.SpinnerStyle.Render("syncing") case StatusDone: - ref := PRRef(item) - if ref == "" { - ref = pastTense(item.Action) + detail := pastTense(item.Action) + if ref := PRRef(item); ref != "" { + detail = ref + " " + detail } - return styles.DoneStyle.Render("✓"), styles.DoneStyle.Render(ref) + return styles.DoneStyle.Render("✓"), styles.DoneStyle.Render(detail) case StatusError: detail := "failed" if item.Error != nil { @@ -218,6 +218,34 @@ func FormatURLSummary(items []Item) string { return "Pull requests\n\n" + strings.Join(rows, "\n") } +// hyperlink wraps text in an OSC 8 terminal hyperlink escape sequence. +// Terminals without OSC 8 support ignore the sequence and show the plain text. +func hyperlink(url, text string) string { + return "\x1b]8;;" + url + "\x1b\\" + text + "\x1b]8;;\x1b\\" +} + +// FormatLinkedURLSummary renders the post-submit PR list as one clickable +// OSC 8 hyperlink per PR. Only for terminal output — non-TTY consumers should +// use FormatURLSummary, which keeps raw URLs greppable. +func FormatLinkedURLSummary(items []Item) string { + rows := make([]string, 0, len(items)) + for _, item := range items { + if item.URL == "" { + continue + } + ref := PRRef(item) + if ref == "" { + ref = "-" + } + name := DisplayBranchName(item.BranchName) + rows = append(rows, hyperlink(item.URL, fmt.Sprintf("%s %s", ref, name))) + } + if len(rows) == 0 { + return "" + } + return "Pull requests\n\n" + strings.Join(rows, "\n") +} + // FormatFailureSummary renders failed branches with their errors. The progress // view is cleared when the TUI exits, so failures must be re-emitted as // persistent output or they vanish from the terminal. @@ -238,16 +266,3 @@ func FormatFailureSummary(items []Item) string { } return strings.Join(rows, "\n") } - -// FormatCompletionSummary renders the persistent post-submit output: PR URLs -// for submitted branches followed by errors for failed ones. -func FormatCompletionSummary(items []Item) string { - sections := make([]string, 0, 2) - if urls := FormatURLSummary(items); urls != "" { - sections = append(sections, urls) - } - if failures := FormatFailureSummary(items); failures != "" { - sections = append(sections, failures) - } - return strings.Join(sections, "\n\n") -} diff --git a/internal/tui/components/submit/format_test.go b/internal/tui/components/submit/format_test.go index 9e6f329ae..2f9765218 100644 --- a/internal/tui/components/submit/format_test.go +++ b/internal/tui/components/submit/format_test.go @@ -31,7 +31,7 @@ func TestFormatCompactRowOmitsInlineURL(t *testing.T) { }, 60, "", DefaultStyles()) require.Contains(t, row, "guard-runner") - require.Contains(t, row, "#934") + require.Contains(t, row, "#934 updated") require.NotContains(t, row, "https://github.com/getstackit/stackit/pull/934") require.LessOrEqual(t, lipgloss.Width(stripANSIEscape(row)), 60) } @@ -54,10 +54,27 @@ func TestFormatURLSummaryRendersClickableRows(t *testing.T) { https://github.com/getstackit/stackit/pull/934`, summary) } -func TestFormatCompletionSummaryIncludesFailures(t *testing.T) { +func TestFormatLinkedURLSummaryEmitsHyperlinks(t *testing.T) { t.Parallel() - summary := FormatCompletionSummary([]Item{ + summary := FormatLinkedURLSummary([]Item{ + { + BranchName: "jonnii/20260511011552/add-feature", + Action: ActionCreate, + Status: StatusDone, + URL: "https://github.com/getstackit/stackit/pull/935", + }, + }) + + require.Equal(t, "Pull requests\n\n"+ + "\x1b]8;;https://github.com/getstackit/stackit/pull/935\x1b\\#935 add-feature\x1b]8;;\x1b\\", + summary) +} + +func TestModelCompletionSummaryIncludesFailuresAndWarnings(t *testing.T) { + t.Parallel() + + m := NewModel([]Item{ { BranchName: "jonnii/20260511011552/add-feature", Action: ActionCreate, @@ -71,19 +88,23 @@ func TestFormatCompletionSummaryIncludesFailures(t *testing.T) { Error: errors.New("failed to push branch: remote rejected"), }, }) + updated, _ := m.Update(WarningMsg{ + BranchName: "jonnii/20260511011552/add-feature", + Warning: "failed to add labels", + }) + m = updated.(*Model) - require.Equal(t, `Pull requests - -#935 add-feature - https://github.com/getstackit/stackit/pull/935 - -✗ fix-bug — failed to push branch: remote rejected`, summary) + summary := m.completionSummary() + require.Contains(t, summary, "Pull requests") + require.Contains(t, summary, "#935 add-feature") + require.Contains(t, summary, "✗ fix-bug — failed to push branch: remote rejected") + require.Contains(t, summary, "⚠️ add-feature: failed to add labels") } -func TestFormatCompletionSummaryFailuresWithoutURLs(t *testing.T) { +func TestFormatFailureSummaryWithoutErrorDetail(t *testing.T) { t.Parallel() - summary := FormatCompletionSummary([]Item{ + summary := FormatFailureSummary([]Item{ { BranchName: "jonnii/20260511011552/fix-bug", Action: ActionUpdate, @@ -125,11 +146,9 @@ func TestModelCompletionSummaryFallsBackToPlanView(t *testing.T) { {BranchName: "feature-1", Action: ActionUpdate, Status: StatusPending, IsSkipped: true, SkipReason: "no changes"}, {BranchName: "feature-2", Action: ActionCreate, Status: StatusPending}, }) - updated, _ := m.Update(GlobalMessageMsg("Dry run complete")) - m = updated.(*Model) summary := stripANSIEscape(m.completionSummary()) - require.Contains(t, summary, "Dry run complete") + require.Contains(t, summary, "Submitting 2 branches") require.Contains(t, summary, "feature-1") require.Contains(t, summary, "no changes") require.Contains(t, summary, "feature-2") diff --git a/internal/tui/components/submit/model.go b/internal/tui/components/submit/model.go index 0c14b1baa..6d08c51df 100644 --- a/internal/tui/components/submit/model.go +++ b/internal/tui/components/submit/model.go @@ -16,9 +16,9 @@ import ( type Model struct { core.BaseModel // Embedded for ReadySignaler interface Items []Item + Warnings []string // formatted warning lines, rendered after the rows and persisted on exit spinner spinner.Model // lowercase for custom style Styles Styles - GlobalMessage string } // ProgressUpdateMsg is sent to update the status of a specific branch submission @@ -29,8 +29,12 @@ type ProgressUpdateMsg struct { Err error } -// GlobalMessageMsg is sent to display a global message (e.g., "Submitting...") -type GlobalMessageMsg string +// WarningMsg surfaces a non-fatal warning for a branch (e.g. labels could not +// be applied). Warnings render below the progress rows and persist on exit. +type WarningMsg struct { + BranchName string + Warning string +} // ProgressCompleteMsg is sent when all submissions are finished type ProgressCompleteMsg struct{} @@ -70,8 +74,8 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } switch msg := msg.(type) { - case GlobalMessageMsg: - m.GlobalMessage = string(msg) + case WarningMsg: + m.Warnings = append(m.Warnings, fmt.Sprintf("⚠️ %s: %s", DisplayBranchName(msg.BranchName), msg.Warning)) return m, nil case ProgressUpdateMsg: @@ -134,34 +138,42 @@ func (m *Model) content() string { } } + for _, warning := range m.Warnings { + b.WriteString("\n") + b.WriteString(warning) + } + return b.String() } // completionSummary is the output persisted to the terminal when the TUI // exits. After a submission it lists PR URLs and failures; when nothing was // submitted (dry run, all up to date) it falls back to the final plan view, -// which would otherwise be erased with the progress display. +// which would otherwise be erased with the progress display. Warnings are +// appended in either case so they survive the screen clear. func (m *Model) completionSummary() string { - if summary := FormatCompletionSummary(m.Items); summary != "" { - return summary + summary := FormatLinkedURLSummary(m.Items) + if failures := FormatFailureSummary(m.Items); failures != "" { + if summary != "" { + summary += "\n\n" + } + summary += failures } - return m.content() + if summary == "" { + return m.content() + } + if len(m.Warnings)> 0 { + summary += "\n\n" + strings.Join(m.Warnings, "\n") + } + return summary } func (m *Model) header() string { - message := strings.TrimSpace(m.GlobalMessage) count := len(m.Items) - if message == "" { - if count == 0 { - return "" - } - return fmt.Sprintf("Submit %d %s", count, pluralBranch(count)) - } - - if strings.TrimSuffix(message, "...") == "Submitting" { - return fmt.Sprintf("Submitting %d %s", count, pluralBranch(count)) + if count == 0 { + return "" } - return message + return fmt.Sprintf("Submitting %d %s", count, pluralBranch(count)) } func pluralBranch(count int) string { diff --git a/internal/tui/stories.go b/internal/tui/stories.go index 1588499eb..9af30b3e1 100644 --- a/internal/tui/stories.go +++ b/internal/tui/stories.go @@ -405,7 +405,6 @@ func registerSubmitStories() { {BranchName: "feature-2", Action: "update", Status: submit.StatusError, Error: fmt.Errorf("failed to push branch: remote rejected")}, {BranchName: "feature-3", Action: "create", Status: submit.StatusPending}, }) - m.GlobalMessage = "Submitting 3 branches..." return m }, }) @@ -451,21 +450,16 @@ func (m *submitSimulationModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch m.step { case 1: - // The TUI only exists during the submission phase; the stack and - // plan print as plain lines before it starts. - _, c := m.submitModel.Update(submit.GlobalMessageMsg("Submitting...")) - cmds = append(cmds, c, m.nextTick()) - case 2: _, c := m.submitModel.Update(submit.ProgressUpdateMsg{BranchName: "feature-2", Status: submit.StatusSubmitting}) cmds = append(cmds, c, m.nextTick()) - case 3: + case 2: m.submitModel.Update(submit.ProgressUpdateMsg{BranchName: "feature-2", Status: submit.StatusDone, URL: "https://github.com/owner/repo/pull/102"}) _, c := m.submitModel.Update(submit.ProgressUpdateMsg{BranchName: "feature-3", Status: submit.StatusSubmitting}) cmds = append(cmds, c, m.nextTick()) - case 4: + case 3: _, c := m.submitModel.Update(submit.ProgressUpdateMsg{BranchName: "feature-3", Status: submit.StatusDone, URL: "https://github.com/owner/repo/pull/103"}) cmds = append(cmds, c, m.nextTick()) - m.submitModel.Update(submit.GlobalMessageMsg("✓ All branches submitted")) + // The final state stays visible until the reset tick. // We don't send ProgressCompleteMsg because it would trigger tea.Quit, // and we don't set Done because the view blanks once Done is set. cmds = append(cmds, tea.Tick(3*time.Second, func(_ time.Time) tea.Msg { @@ -489,6 +483,6 @@ func (m *submitSimulationModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { func (m *submitSimulationModel) View() tea.View { helpStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("241")).MarginTop(1) return tea.NewView(fmt.Sprint(m.submitModel.View().Content) + "\n" + - helpStyle.Render(fmt.Sprintf("Simulation step: %d/4", m.step)) + "\n" + + helpStyle.Render(fmt.Sprintf("Simulation step: %d/3", m.step)) + "\n" + helpStyle.Render("q: back")) }

AltStyle によって変換されたページ (->オリジナル) /