Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixed-signoff-stall-note-and-log-level.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The sign-off attribution notes on an adopted or hive-caused `stage/085` PR (`adoption-stalled:hive-signoff-attribution` and the maintainer sign-off note) no longer say the `Sign-off survives the squash` gate "stays red" on repositories that do not run `.github/workflows/dco-squash-attribution.yml`. They now say the hive will not squash-merge the head, and name that check only when the head's required or reported checks include it. The adopted-PR stall escalation ("adopted PR sits with nothing the hive can push") is logged at INFO instead of WARN, so designed escalations no longer add up to an `unclassified_error_repeated` fleet self-report ([#14466](https://github.com/hivecommons/hive/issues/14466)).
6 changes: 5 additions & 1 deletion src/docs/labels-and-control-signals.md
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,11 @@ The split also covers a head whose commits GitHub links to the hive App while
the PR author is someone else — a hive follow-up commit on the branch, or a
contributor who committed from inside a hive pod with the App's noreply
address (hivecommons/pluk#315 and the other OWNERS PRs of 2026-10-20). The
required "Sign-off survives the squash" check is red there, and no hive push
hive will not squash-merge such a head. On a repository that runs
`.github/workflows/dco-squash-attribution.yml`, the "Sign-off survives the
squash" check is red there too. The hive's notes name that check only when
the head's required or reported checks include it, because most managed
repositories do not carry the workflow. No hive push
lands on a non-App-authored branch (`review.fix_human_prs` does not authorize
one and the hive must not forge the author's `Signed-off-by`). Who put
those commits there decides the route. When the hive did — the
Expand Down
2 changes: 2 additions & 0 deletions src/pkg/github/automerge/automerge_adoption_escape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ func TestAdoptedInPlaceSquashUnsafeHiveCommitStallsWithoutSupersede(t *testing.T
h.adopt.setCommitAuthor("f72fa44", testHiveAppBotLogin)

h.clock.advance(time.Minute)
// This repo runs the gate (dco-squash-attribution.yml), so the note names it.
h.e.noteStageDCOGate("acme", "widget", 7, "f72fa44", map[string]bool{squashSignoffCheck: true}, nil)
h.observe(adoptPR(7, "f72fa44", adoptedLabels...), contributorLaneReasonNoVerdict)
if h.adopt.refAttempts != 0 || len(h.adopt.refs) != 0 || len(h.adopt.pulls) != 0 {
t.Fatalf("signoff supersede opened: attempts=%d refs=%v pulls=%v", h.adopt.refAttempts, h.adopt.refs, h.adopt.pulls)
Expand Down
6 changes: 3 additions & 3 deletions src/pkg/github/automerge/automerge_contributor_adopt.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,9 +452,9 @@ func adoptionBlockerText(obs stageObservation, rec stagelabels.Record) string {
switch {
case adoptedHeadSignoffUnfixable(obs, rec):
author := nonEmpty(obs.Author, "the contributor")
msg := fmt.Sprintf("%d commit(s) on `%s` carry the App's `Signed-off-by` while a squash merge would land them as @%s, so the `Sign-off survives the squash` gate stays red. "+
msg := fmt.Sprintf("%d commit(s) on `%s` carry the App's `Signed-off-by` while a squash merge would land them as @%s, so the hive will not squash-merge this head%s. "+
"The hive cannot rewrite that authorship without the contributor, and a superseding PR would carry the same head, so it opened none. ",
obs.Detail.DCOSquashUnsafeHive, shortSHA(obs.HeadSHA), author)
obs.Detail.DCOSquashUnsafeHive, shortSHA(obs.HeadSHA), author, squashSignoffGateNote(obs))
msg += mergeCommitRefusalText(obs)
return msg
case verdictBlocks(obs.Detail):
Expand Down Expand Up @@ -1176,7 +1176,7 @@ func (c *Engine) escapeStalledAdoption(ctx context.Context, store *stagelabels.S
}
noted := rec.AdoptionEscalationNoted(obs.HeadSHA)
store.MarkAdoptionEscalated(key, obs.HeadSHA, reason)
c.warn("stage labels: adopted PR sits with nothing the hive can push; escalating to needs-decision", "repo", obs.Display, "pr", obs.Number, "head", obs.HeadSHA, "reason", reason, "head_since", rec.AdoptedHeadSince(), "noted", noted)
c.info("stage labels: adopted PR sits with nothing the hive can push; escalating to needs-decision", "repo", obs.Display, "pr", obs.Number, "head", obs.HeadSHA, "reason", reason, "head_since", rec.AdoptedHeadSince(), "noted", noted)
if noted {
// The blocker flapped on a head already noted: re-escalate silently.
return adoptionEscape{Escalated: true, Reason: reason}, comments, firstErr
Expand Down
25 changes: 23 additions & 2 deletions src/pkg/github/automerge/automerge_merge_commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,27 @@ func (c *Engine) noteMergeCommit(ctx context.Context, store *stagelabels.Store,
return 1, nil
}

// squashSignoffCheck is the check-run name of
// .github/workflows/dco-squash-attribution.yml.
const squashSignoffCheck = "Sign-off survives the squash"

func isSquashSignoffCheck(name string) bool {
return strings.EqualFold(strings.TrimSpace(name), squashSignoffCheck)
}

// squashSignoffGateNote is the sign-off attribution notes' mention of that
// check, or "" when the head's required and reported checks do not include
// it (noteStageDCOGate): the workflow lives in the repositories that carry
// it, not in every repository the hive manages, and the hive's own commit
// check is what holds the head everywhere else.
func squashSignoffGateNote(obs stageObservation) string {
d := obs.Detail
if !d.SquashSignoffGate || (d.DCOGateHead != "" && obs.HeadSHA != "" && d.DCOGateHead != obs.HeadSHA) {
return ""
}
return " (the `" + squashSignoffCheck + "` check stays red)"
}

// SignoffAttributionDecisionMarkerPrefix opens the one stage/085 note per
// head for a hive-caused sign-off attribution the lane could not resolve
// with a merge commit by itself.
Expand All @@ -158,8 +179,8 @@ func SignoffAttributionDecisionComment(obs stageObservation) string {
author := nonEmpty(obs.Author, "the contributor")
var b strings.Builder
fmt.Fprintf(&b, "%s head=%s -->\n", SignoffAttributionDecisionMarkerPrefix, obs.HeadSHA)
fmt.Fprintf(&b, "Held at `%s` for a maintainer. %d of %d commits on `%s` are the hive's own, signed off as the hive App, so a squash merge would land them as @%s with the App's sign-off and the `Sign-off survives the squash` gate stays red. ",
stagelabels.NeedsDecision.Label(), n, total, shortSHA(obs.HeadSHA), author)
fmt.Fprintf(&b, "Held at `%s` for a maintainer. %d of %d commits on `%s` are the hive's own, signed off as the hive App, so a squash merge would land them as @%s with the App's sign-off, and the hive will not squash-merge this head%s. ",
stagelabels.NeedsDecision.Label(), n, total, shortSHA(obs.HeadSHA), author, squashSignoffGateNote(obs))
fmt.Fprintf(&b, "@%s did not make those commits and has nothing to fix here.\n\n", author)
b.WriteString(mergeCommitRefusalText(obs))
return b.String()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ type signoffCase struct {
squashOnly bool
// approvedHead, when set, is the head a trusted maintainer approved.
approvedHead string
// squashGate: the head's checks include `Sign-off survives the squash`.
squashGate bool
}

func runSignoffCase(t *testing.T, tc signoffCase) (*stageAPI, stagelabels.Record, *Engine, func()) {
Expand Down Expand Up @@ -66,6 +68,9 @@ func runSignoffCase(t *testing.T, tc signoffCase) (*stageAPI, stagelabels.Record
pr.User = &gh.User{Login: gh.Ptr("j1mc")}
e.recordStage(stageLaneContributor, "acme/widget", "acme", "widget", pr, tc.reason, false, nil)
e.noteStageRole("acme", "widget", n, stagelabels.RoleContributor)
if tc.squashGate {
e.noteStageDCOGate("acme", "widget", n, pr.GetHead().GetSHA(), nil, map[string]bool{squashSignoffCheck: true})
}
if _, err := e.ApplyStageLabels(context.Background()); err != nil {
t.Fatal(err)
}
Expand Down
144 changes: 144 additions & 0 deletions src/pkg/github/automerge/automerge_signoff_gate_note_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
package automerge

import (
"bytes"
"log/slog"
"strings"
"sync"
"testing"
"time"

"github.com/hivecommons/hive/pkg/fleetreport/slogfeed"
"github.com/hivecommons/hive/pkg/stagelabels"
)

// hivecommons/hive#14466: the projectbluefin hive parked utah-packages#660,
// chairlift#638 and testsuite#1028 at 085 on hive-signoff-attribution after
// its own fix commit. The park is designed; two things around it were not.
// Its notes said the `Sign-off survives the squash` gate stays red, but that
// check is a workflow only some repositories carry and none of those three
// ran it. And the escalation was logged at WARN, so the slog feed counted
// three designed parks toward an unclassified_error_repeated self-report.

const escalationLogMsg = "stage labels: adopted PR sits with nothing the hive can push"

// squashUnsafeAdoption adopts PR 7 in place and gives head 021bb6f the shape
// of all three PRs: the contributor's own signed commit, then the hive's fix
// signed off as the App. The caller observes it once to trigger the stall.
func squashUnsafeAdoption(t *testing.T) (*adoptHarness, func()) {
t.Helper()
p := adoptPolicy()
p.AdoptInPlace = true
h, done := newAdoptHarness(t, ContributorNudgeOutcome{Mode: "comment"}, p)
h.api.commitDetails = map[int][]stageCommitFixture{7: {
{message: "fix(smoke): restore default app checks\n\nSigned-off-by: thatguywob <thatguywob@users.noreply.github.com>", authorLogin: "thatguywob", authorName: "thatguywob", authorEmail: "thatguywob@users.noreply.github.com"},
{message: "fix(ci): preload apps for MIME checks\n\nSigned-off-by: hive[bot] <12345+hive[bot]@users.noreply.github.com>", authorLogin: testHiveAppBotLogin, authorName: "hive[bot]", authorEmail: "12345+hive[bot]@users.noreply.github.com"},
}}
adoptInPlaceAt(h, "021bb6f", contributorLaneReasonNoVerdict)
h.adopt.setCommitAuthor("021bb6f", testHiveAppBotLogin)
h.clock.advance(time.Minute)
return h, done
}

func TestSignoffStallNoteNamesSquashGateOnlyWhereItRuns(t *testing.T) {
for _, tc := range []struct {
name string
gate bool
}{
{"repository without dco-squash-attribution.yml", false},
{"repository running it", true},
} {
t.Run(tc.name, func(t *testing.T) {
h, done := squashUnsafeAdoption(t)
defer done()
if tc.gate {
h.e.noteStageDCOGate("acme", "widget", 7, "021bb6f", map[string]bool{squashSignoffCheck: true}, nil)
}
h.observe(adoptPR(7, "021bb6f", adoptedLabels...), contributorLaneReasonNoVerdict)
notes := stalledComments(h.api)
if len(notes) != 1 {
t.Fatalf("stalled note: %v", h.api.comments)
}
for _, want := range []string{"so the hive will not squash-merge this head", "`gh pr merge 7 --merge`, not squash"} {
if !strings.Contains(notes[0], want) {
t.Fatalf("note missing %q:\n%s", want, notes[0])
}
}
if named := strings.Contains(notes[0], squashSignoffCheck); named != tc.gate {
t.Fatalf("note names the %q check = %v, want %v:\n%s", squashSignoffCheck, named, tc.gate, notes[0])
}
if strings.Contains(notes[0], "gate stays red") {
t.Fatalf("note still uses the old wording:\n%s", notes[0])
}
})
}
}

func TestSignoffAttributionDecisionNoteNamesSquashGateOnlyWhereItRuns(t *testing.T) {
for _, gate := range []bool{false, true} {
api, rec, _, _ := runSignoffCase(t, signoffCase{commits: testsuite991Commits, reason: contributorLaneReasonNoVerdict, squashGate: gate})
if rec.Stage != stagelabels.NeedsDecision || rec.Reason != stagelabels.LaneReasonHiveSignoffAttribution {
t.Fatalf("gate=%v: record %+v, want 085 on hive-signoff-attribution", gate, rec)
}
notes := commentsWith(api, SignoffAttributionDecisionMarkerPrefix)
if len(notes) != 1 {
t.Fatalf("gate=%v: maintainer note: %q", gate, api.comments)
}
if !strings.Contains(notes[0], "the hive will not squash-merge this head") || strings.Contains(notes[0], "gate stays red") {
t.Fatalf("gate=%v: note wording:\n%s", gate, notes[0])
}
if named := strings.Contains(notes[0], squashSignoffCheck); named != gate {
t.Fatalf("note names the %q check = %v, want %v:\n%s", squashSignoffCheck, named, gate, notes[0])
}
}
}

// The check seen on an earlier head says nothing about this one.
func TestSquashSignoffGateNoteIsPerHead(t *testing.T) {
obs := stageObservation{HeadSHA: "new"}
obs.Detail.SquashSignoffGate, obs.Detail.DCOGateHead = true, "old"
if got := squashSignoffGateNote(obs); got != "" {
t.Fatalf("gate seen on another head: got %q, want none", got)
}
obs.Detail.DCOGateHead = "new"
if got := squashSignoffGateNote(obs); !strings.Contains(got, squashSignoffCheck) {
t.Fatalf("gate seen on this head: got %q", got)
}
}

// The escalation is logged at INFO, so the slog feed (WARN and above) never
// counts it toward a self-report. The feed here is the real handler.
func TestAdoptionStallEscalationLogsAtInfo(t *testing.T) {
h, done := squashUnsafeAdoption(t)
defer done()
var buf bytes.Buffer
var mu sync.Mutex
var fed []string
h.e.logger = slog.New(slogfeed.NewWithObserver(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}), func(_, _, message, _ string) {
mu.Lock()
defer mu.Unlock()
fed = append(fed, message)
}))

h.observe(adoptPR(7, "021bb6f", adoptedLabels...), contributorLaneReasonNoVerdict)
if len(stalledComments(h.api)) != 1 {
t.Fatalf("expected the escalation to happen: %v", h.api.comments)
}
var line string
for _, l := range strings.Split(buf.String(), "\n") {
if strings.Contains(l, escalationLogMsg) {
line = l
break
}
}
if !strings.Contains(line, "level=INFO") {
t.Fatalf("escalation log line %q, want level=INFO", line)
}
mu.Lock()
defer mu.Unlock()
for _, m := range fed {
if strings.Contains(m, escalationLogMsg) {
t.Fatalf("the slog feed counted the designed escalation toward a self-report: %q", m)
}
}
}
12 changes: 9 additions & 3 deletions src/pkg/github/automerge/automerge_stage_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ func (t *StageTracker) drain() [][]stageObservation {
}
if detail.DCOGateHead != "" && (o.HeadSHA == "" || detail.DCOGateHead == o.HeadSHA) {
o.Detail.DCOGate, o.Detail.DCOGateHead = detail.DCOGate, detail.DCOGateHead
o.Detail.SquashSignoffGate = detail.SquashSignoffGate
}
list = append(list, o)
}
Expand Down Expand Up @@ -550,21 +551,26 @@ func stageActivity(obs stageObservation, stage stagelabels.Stage) stagelabels.Ac
// (hivecommons/hive#12542): in the required-check set or reported on the
// head by the DCO App / Prow's plugin. It is the detected "this repo
// enforces DCO" signal; a head without one is not routed to 072 for a
// missing sign-off unless config says the repo requires it.
// missing sign-off unless config says the repo requires it. It also records
// whether the `Sign-off survives the squash` check is among them, which only
// the sign-off attribution notes read.
func (c *Engine) noteStageDCOGate(owner, repo string, number int, head string, required map[string]bool, observed map[string]bool) {
if !c.stagePolicy().Enabled || number <= 0 {
return
}
gate := false
gate, squashGate := false, false
for _, set := range []map[string]bool{required, observed} {
for name := range set {
if hgithub.IsDCOContext(name) {
gate = true
}
if isSquashSignoffCheck(name) {
squashGate = true
}
}
}
c.stageTrackerOrShared().note(stageKey(owner, repo, number), func(d *stageDetail) {
d.DCOGate, d.DCOGateHead = gate, head
d.DCOGate, d.DCOGateHead, d.SquashSignoffGate = gate, head, squashGate
})
}

Expand Down
6 changes: 6 additions & 0 deletions src/pkg/stageengine/prstage/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ type Detail struct {
// no per-repo override is configured.
DCOGate bool
DCOGateHead string
// SquashSignoffGate records, for the same head as DCOGateHead, whether
// the `Sign-off survives the squash` check
// (.github/workflows/dco-squash-attribution.yml) is required or reported
// there. Only repositories that carry that workflow run it, so the
// sign-off attribution notes name it only where it is.
SquashSignoffGate bool
}

// RedCheck returns the first red non-required check on head, or "".
Expand Down
Loading