🐛 Approve toast: outcome in plain words, who merges next, sentinel only while open

changelog.d/fixed-approve-toast-wording.md

@@ -0,0 +1 @@
+- The dashboard's Approve result toast now leads with the outcome in plain words ("Approved #510 as @user · sentinel alert cleared by your review"), names the concrete next step and who merges (hive's merge sweep with the soak figure, or "merge it on GitHub yourself" on the human-merge lane), repeats the ⚠ sentinel paragraph only while the alert is still open after the action, lists what was posted on a muted secondary line, and on Prow-managed repos says the approval is recorded in hive while Prow approval still needs an OWNERS reviewer — instead of "`/lgtm` + `/approve` posted (released: …) … the merge gate takes it from here; the dashboard merges nothing" ([#15212](https://github.com/hivecommons/hive/issues/15212)).

src/pkg/dashboard/api_repo_item_decide_delegated.go

@@ -732,7 +732,7 @@ func (s *Server) executeDelegatedDecision(ctx context.Context, in delegatedecisi
 				if _, err := client.RecordOperatorPullRequestApproval(ctx, repoFull, number, user); err != nil {
 					return notes, fmt.Errorf("approving PR: %w", err)
 				}
-				notes = append(notes, "`/lgtm` + `/approve` recorded; the merge gate takes it from here")
+				notes = append(notes, "`/lgtm` + `/approve` recorded; hive's merge sweep merges it once CI is green and the head has soaked")
 			}
 		case delegatedecision.ActionLabelsAdd:
 			for _, l := range a.Labels {

src/pkg/dashboard/api_repo_item_decision.go

@@ -1,3 +1,3 @@
 package dashboard
 
 import (
@@ -189,20 +189,37 @@
 	return ""
 }
 
+// repoItemApproveFacts is what a PR Approve has to say about itself:
+// who approved, what the approval released (repoItemApprovalReleased plus
+// "admitted"), the stage label the engine derived on the spot ("" when
+// deferred to the sweep), whether the sentinel alert is still open, and
+// how long the merge sweep soaks a head before merging.
+type repoItemApproveFacts struct {
+	User          string
+	Released      []string
+	Stage         string
+	SentinelOpen  bool
+	SentinelLabel string
+	SoakMinutes   int
+	// Prow says the PR is on a Prow-managed repo: the relayed `/approve`
+	// came from the hive's account, which is not in OWNERS, so Prow ignores
+	// it and the toast must not claim the PR is approved for Prow.
+	Prow bool
+}
+
 // repoItemDecisionResult reads a recorded human decision into the shared
 // row-action result.
 func repoItemDecisionResult(repoFull string, number int, itemType, decision, comment, supersededBy, label string, released []string) *RowActionResult {
+	return repoItemDecisionResultWith(repoFull, number, itemType, decision, comment, supersededBy, label, repoItemApproveFacts{Released: released})
+}
+
+func repoItemDecisionResultWith(repoFull string, number int, itemType, decision, comment, supersededBy, label string, facts repoItemApproveFacts) *RowActionResult {
 	out := &RowActionResult{Target: rowActionTarget(repoFull, itemType, number)}
 	switch decision {
 	case repoItemDecisionApprove:
 		out.Action = rowActionApproved
 		if itemType == "pr" {
-			out.Outcome = "`/lgtm` + `/approve` posted"
-			if len(released) > 0 {
-				out.Outcome += " (released: " + strings.Join(released, ", ") + ")"
-			}
-			out.Outcome += "."
-			out.Next = "The merge gate takes it from here; the dashboard merges nothing."
+			repoItemApproveResult(out, number, facts)
 		} else {
 			out.Outcome = "`/hive approve` recorded; parking labels lifted, `" + label + "` applied."
 			out.Next = "The hive un-parks the issue and carries on; nothing is merged."
@@ -242,7 +259,7 @@
 	case repoItemDecisionQueueMerge:
 		out.Action = rowActionQueued
 		out.Outcome = "Your approval is recorded; `" + label + "` applied."
-		out.Next = "Hive's merge sweep merges it on the next tick once CI is green, respecting soak; the dashboard merges nothing itself."
+		out.Next = "Hive's merge sweep merges it on its next pass once CI is green and the head has soaked."
 	}
 	return out
 }
@@ -555,6 +572,7 @@
 	)
 	var released []string
 	stage := ""
+	sentinelOpen, prow := false, false
 	if decision == repoItemDecisionApprove {
 		var event ghpkg.PullRequestApprovalEvent
 		event, err = gh.RecordOperatorPullRequestApprovalWith(ctx, repoFull, number, user, ghpkg.PullRequestApprovalOptions{
@@ -563,7 +581,7 @@
 			SentinelLabel:       s.sentinelLabel(),
 		})
 		head = event.HeadSHA
-		released, stage = repoItemApprovalReleased(event), event.Stage
+		released, stage, sentinelOpen, prow = repoItemApprovalReleased(event), event.Stage, event.SentinelLabelOpen, event.ProwManaged
 	} else {
 		var event ghpkg.PullRequestDecisionEvent
 		event, err = gh.RecordOperatorPullRequestDecision(ctx, repoFull, number, user, comment, repoItemDecisionChangeRequests())
@@ -603,7 +621,9 @@
 	// next tracker read instead of the cached 085.
 	s.invalidateStageTrackerForWebhook()
 	jsonResponse(w, repoItemDecisionResponse{OK: true, Repo: repoFull, Number: number, Decision: decision, Status: status, Released: released, Stage: stage, Head: head, AdmittedBy: admitted,
-		Result: repoItemDecisionResult(repoFull, number, "pr", decision, comment, "", "", released)})
+		Result: repoItemDecisionResultWith(repoFull, number, "pr", decision, comment, "", "", repoItemApproveFacts{
+			User: user, Released: released, Stage: stage, SentinelOpen: sentinelOpen, SentinelLabel: s.sentinelLabel(), SoakMinutes: s.approveSoakMinutes(admitted != ""), Prow: prow,
+		})})
 }
 
 // repoItemApprovalReleased names what a PR approval released, for the
@@ -624,3 +644,85 @@
 	}
 	return out
 }
+
+// repoItemApproveResult words a PR Approve the way the owner reads it
+// (hivecommons/dibs#510 read "`/lgtm` + `/approve` posted (released:
+// approval-recorded, sentinel-label)" followed by the ⚠ sentinel warning
+// it had just released and "the dashboard merges nothing"): the headline
+// is the outcome in plain words, Next is the concrete next step and who
+// merges, and what was posted rides Posted as the muted secondary line.
+func repoItemApproveResult(out *RowActionResult, number int, facts repoItemApproveFacts) {
+	released := map[string]bool{}
+	for _, r := range facts.Released {
+		released[r] = true
+	}
+	headline := "Approved #" + strconv.Itoa(number)
+	if facts.Prow {
+		headline = "Approval of #" + strconv.Itoa(number) + " recorded in hive"
+	}
+	if u := strings.TrimSpace(facts.User); u != "" {
+		headline += " as @" + u
+	}
+	out.SentinelCleared = released["sentinel-label"]
+	out.SentinelOpen = facts.SentinelOpen && !out.SentinelCleared
+	switch {
+	case out.SentinelCleared:
+		headline += " · sentinel alert cleared by your review"
+	case out.SentinelOpen:
+		headline += " · sentinel alert still open"
+	}
+	out.Outcome = headline + "."
+	out.Posted = append(out.Posted, "`/lgtm` posted", "`/approve` posted")
+	if released["needs-human"] {
+		out.Posted = append(out.Posted, "`needs-human` label removed")
+	}
+	if out.SentinelCleared {
+		out.Posted = append(out.Posted, "`"+repoItemSentinelLabelOrDefault(facts.SentinelLabel)+"` label removed")
+	}
+	if released["admitted"] {
+		out.Posted = append(out.Posted, "head admitted as-is on your approval")
+	}
+	switch {
+	case out.SentinelOpen:
+		out.Next = "The `" + repoItemSentinelLabelOrDefault(facts.SentinelLabel) + "` label is still on the PR: a maintainer must review the flagged head and take the label off on GitHub before hive's merge sweep will touch it."
+	case facts.Stage == stagelabels.ReadyForHumanMerge.Label():
+		out.Next = "Hive cannot merge this one (trust or role blocks it): merge it on GitHub yourself, or use Approve & merge as maintainer."
+	default:
+		out.Next = "Hive's merge sweep merges it on its next pass once CI is green and the head has soaked" + repoItemSoakNote(facts.SoakMinutes) + "."
+	}
+	if facts.Prow {
+		// Prow reads `/approve` only from OWNERS; the hive's account is not
+		// one, so the relayed command is "NOT APPROVED" there.
+		out.Next = "Prow approval still needs an OWNERS reviewer: the relayed `/approve` came from hive's account, which Prow does not count. " + out.Next
+	}
+}
+
+func repoItemSentinelLabelOrDefault(label string) string {
+	if strings.TrimSpace(label) == "" {
+		return config.DefaultSentinelLabel
+	}
+	return strings.TrimSpace(label)
+}
+
+func repoItemSoakNote(minutes int) string {
+	if minutes <= 0 {
+		return ""
+	}
+	return " (≈" + strconv.Itoa(minutes) + " min)"
+}
+
+// approveSoakMinutes is how long the merge sweep soaks an approved head:
+// the contributor lane's soak when the approval admitted a foreign head,
+// otherwise the sweep's minimum head age.
+func (s *Server) approveSoakMinutes(admitted bool) int {
+	if s == nil || s.deps == nil || s.deps.Config == nil {
+		if admitted {
+			return config.DefaultContributorLaneSoakMinutes
+		}
+		return int(config.DefaultAutoMergeMinHeadAge / time.Minute)
+	}
+	if admitted {
+		return s.deps.Config.AutoMerge.ContributorLane.EffectiveSoakMinutes()
+	}
+	return int(s.deps.Config.AutoMerge.EffectiveMinHeadAge() / time.Minute)
+}

src/pkg/dashboard/api_repo_item_decision_test.go

@@ -559,3 +559,83 @@ func TestRepoItemDecisionMergeDecisionsGuard(t *testing.T) {
 		t.Fatalf("guards must stop before GitHub writes: merges=%v reviews=%v", mock.merges, mock.reviews)
 	}
 }
+
+// TestRepoItemApproveResultWording (owner, hivecommons/dibs#510 — "that's
+// a confusing response"): the approve toast leads with the outcome in
+// plain words, says who merges next, flags the sentinel alert only while
+// it is still open, and lists what was posted as a secondary line.
+func TestRepoItemApproveResultWording(t *testing.T) {
+	cases := []struct {
+		name      string
+		facts     repoItemApproveFacts
+		outcome   string
+		next      string
+		posted    string
+		cleared   bool
+		stillOpen bool
+	}{
+		{
+			name:    "sentinel released, lane merges",
+			facts:   repoItemApproveFacts{User: "clubanderson", Released: []string{"approval-recorded", "sentinel-label", "sentinel-acknowledged", "admitted"}, SentinelLabel: "sentinel-alert", SoakMinutes: 30},
+			outcome: "Approved #510 as @clubanderson · sentinel alert cleared by your review.",
+			next:    "Hive's merge sweep merges it on its next pass once CI is green and the head has soaked (≈30 min).",
+			posted:  "`/lgtm` posted|`/approve` posted|`sentinel-alert` label removed|head admitted as-is on your approval",
+			cleared: true,
+		},
+		{
+			name:      "sentinel still open",
+			facts:     repoItemApproveFacts{User: "clubanderson", Released: []string{"approval-recorded"}, SentinelOpen: true, SentinelLabel: "sentinel-alert", SoakMinutes: 30},
+			outcome:   "Approved #510 as @clubanderson · sentinel alert still open.",
+			next:      "The `sentinel-alert` label is still on the PR: a maintainer must review the flagged head and take the label off on GitHub before hive's merge sweep will touch it.",
+			posted:    "`/lgtm` posted|`/approve` posted",
+			stillOpen: true,
+		},
+		{
+			name:    "human must merge",
+			facts:   repoItemApproveFacts{User: "clubanderson", Released: []string{"approval-recorded", "needs-human"}, Stage: stagelabels.ReadyForHumanMerge.Label(), SoakMinutes: 3},
+			outcome: "Approved #510 as @clubanderson.",
+			next:    "Hive cannot merge this one (trust or role blocks it): merge it on GitHub yourself, or use Approve & merge as maintainer.",
+			posted:  "`/lgtm` posted|`/approve` posted|`needs-human` label removed",
+		},
+		{
+			name:    "prow-managed repo: approval recorded, not claimed approved for prow",
+			facts:   repoItemApproveFacts{User: "clubanderson", Released: []string{"approval-recorded"}, SoakMinutes: 3, Prow: true},
+			outcome: "Approval of #510 recorded in hive as @clubanderson.",
+			next:    "Prow approval still needs an OWNERS reviewer: the relayed `/approve` came from hive's account, which Prow does not count. Hive's merge sweep merges it on its next pass once CI is green and the head has soaked (≈3 min).",
+			posted:  "`/lgtm` posted|`/approve` posted",
+		},
+		{
+			name:    "no user, no soak figure",
+			facts:   repoItemApproveFacts{Released: []string{"approval-recorded"}},
+			outcome: "Approved #510.",
+			next:    "Hive's merge sweep merges it on its next pass once CI is green and the head has soaked.",
+			posted:  "`/lgtm` posted|`/approve` posted",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := repoItemDecisionResultWith("hivecommons/dibs", 510, "pr", repoItemDecisionApprove, "", "", "", tc.facts)
+			if got.Action != rowActionApproved || got.Outcome != tc.outcome || got.Next != tc.next {
+				t.Fatalf("result = %+v", got)
+			}
+			if p := strings.Join(got.Posted, "|"); p != tc.posted {
+				t.Fatalf("posted = %q", p)
+			}
+			if got.SentinelCleared != tc.cleared || got.SentinelOpen != tc.stillOpen {
+				t.Fatalf("sentinel flags cleared=%v open=%v", got.SentinelCleared, got.SentinelOpen)
+			}
+			for _, bad := range []string{"released:", "merges nothing", "merge gate takes it"} {
+				if strings.Contains(got.Outcome+got.Next, bad) {
+					t.Fatalf("old wording %q survives: %+v", bad, got)
+				}
+			}
+			if !tc.facts.Prow && strings.Contains(got.Outcome+got.Next, "Prow") {
+				t.Fatalf("non-prow repo mentions Prow: %+v", got)
+			}
+		})
+	}
+	// The legacy builder keeps the same shape for callers without facts.
+	if got := repoItemDecisionResult("o/r", 1, "pr", repoItemDecisionApprove, "", "", "", nil); got.Outcome != "Approved #1." {
+		t.Fatalf("legacy = %+v", got)
+	}
+}

src/pkg/dashboard/row_action_result.go

@@ -1,3 +1,3 @@
 package dashboard
 
 import (
@@ -30,6 +30,15 @@
 	// the popover's shape (hivecommons/hive#14822). Both additive.
 	Why     string                       `json:"why,omitempty"`
 	Options []StageTrackerDecisionOption `json:"options,omitempty"`
+	// Posted lists what the action wrote on GitHub (`/lgtm`, `/approve`,
+	// a label removed) for the toast's muted secondary line; Outcome stays
+	// the plain-words headline. SentinelCleared says the action took the
+	// sentinel alert off, so the toast drops its ⚠ paragraph; SentinelOpen
+	// says the PR still wears the alert after the action and a person must
+	// clear it. Both additive.
+	Posted          []string `json:"posted,omitempty"`
+	SentinelCleared bool     `json:"sentinel_cleared,omitempty"`
+	SentinelOpen    bool     `json:"sentinel_open,omitempty"`
 }
 
 // RowActionTarget names the issue or PR a row action acted on.
@@ -162,9 +171,10 @@
 		case note("queued for auto-merge"):
 			out.Next = "Queued: the auto-merge engine merges per repository settings."
 		case note("/lgtm"):
-			out.Next = "`/lgtm` + `/approve` posted; the merge gate takes it from here."
+			out.Next = "Hive's merge sweep merges it on its next pass once CI is green and the head has soaked."
+			out.Posted = append(out.Posted, "`/lgtm` posted", "`/approve` posted")
 		default:
-			out.Next = "The merge gate takes it from here; the dashboard merges nothing."
+			out.Next = "Hive's merge sweep merges it on its next pass once CI is green and the head has soaked."
 		}
 	}
 	return out