Fix non-zero exit on error and debounce transient mergeable=false
agentpr/watchpr already propagated command errors to a non-zero exit, but that behaviour had no regression coverage and the root command was not constructible outside main(). watchpr also fired a spurious conflict alert because Gitea computes mergeability asynchronously and can briefly report mergeable=false right after a push. The docs additionally printed the AppRole role_id literal UUID. - Extract newRootCmd() in both cmd/agentpr and cmd/watchpr so main() only runs Execute and exits non-zero on error; add tests asserting Execute returns an error for a bad PR ref / malformed --repo / no args. - Debounce mergeability loss in MeaningfulChange: only alert when mergeable=false persists across two consecutive polls (both prev and cur false, still open); update the table test for one-poll-false (benign), false-persisting (alert), and recovered false->true (benign). - Refer to AGENT_APPROLE_ROLE_ID by env var in README.md/AGENTS.md without printing the literal role_id; keep the code default and env override.
This commit is contained in:
@@ -67,7 +67,7 @@ func isFailedCI(state string) bool {
|
||||
// - the PR closed without merging
|
||||
// - a new comment from someone other than the agent
|
||||
// - CI transitioned into failure/error
|
||||
// - the PR lost mergeability (a conflict appeared)
|
||||
// - the PR lost mergeability (a conflict appeared) for two consecutive polls
|
||||
func MeaningfulChange(prev, cur PRState) (bool, string) {
|
||||
if !prev.Merged && cur.Merged {
|
||||
return true, "PR merged"
|
||||
@@ -82,7 +82,11 @@ func MeaningfulChange(prev, cur PRState) (bool, string) {
|
||||
if isFailedCI(cur.CIStatus) && !isFailedCI(prev.CIStatus) {
|
||||
return true, "CI failed (" + cur.CIStatus + ")"
|
||||
}
|
||||
if prev.Mergeable && !cur.Mergeable && cur.State == "open" {
|
||||
// Gitea computes mergeability asynchronously, so a PR can briefly report
|
||||
// mergeable=false right after a push. Require the loss to persist across two
|
||||
// consecutive polls (both prev and cur false, still open) before treating it
|
||||
// as a real conflict; a single false poll is debounced.
|
||||
if !prev.Mergeable && !cur.Mergeable && cur.State == "open" {
|
||||
return true, "PR lost mergeability (conflict)"
|
||||
}
|
||||
return false, ""
|
||||
|
||||
@@ -17,6 +17,7 @@ func base() PRState {
|
||||
func TestMeaningfulChange(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
mutatePrev func(s *PRState)
|
||||
mutate func(s *PRState)
|
||||
wantChange bool
|
||||
}{
|
||||
@@ -56,10 +57,27 @@ func TestMeaningfulChange(t *testing.T) {
|
||||
wantChange: true,
|
||||
},
|
||||
{
|
||||
name: "lost mergeability alerts",
|
||||
// A single mergeable=false poll is debounced: Gitea often reports
|
||||
// this transiently right after a push.
|
||||
name: "mergeable true to false for one poll is benign",
|
||||
mutate: func(s *PRState) { s.Mergeable = false },
|
||||
wantChange: false,
|
||||
},
|
||||
{
|
||||
// mergeable=false persisting into a second consecutive poll is a
|
||||
// real conflict and alerts.
|
||||
name: "mergeable false persisting a second poll alerts",
|
||||
mutatePrev: func(s *PRState) { s.Mergeable = false },
|
||||
mutate: func(s *PRState) { s.Mergeable = false },
|
||||
wantChange: true,
|
||||
},
|
||||
{
|
||||
// mergeable recovered (false then true) must not alert.
|
||||
name: "mergeable recovered false to true is benign",
|
||||
mutatePrev: func(s *PRState) { s.Mergeable = false },
|
||||
mutate: func(s *PRState) {},
|
||||
wantChange: false,
|
||||
},
|
||||
{
|
||||
name: "new head sha alone is benign",
|
||||
mutate: func(s *PRState) { s.HeadSHA = "def456" },
|
||||
@@ -69,6 +87,9 @@ func TestMeaningfulChange(t *testing.T) {
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
prev := base()
|
||||
if tt.mutatePrev != nil {
|
||||
tt.mutatePrev(&prev)
|
||||
}
|
||||
cur := base()
|
||||
tt.mutate(&cur)
|
||||
got, reason := MeaningfulChange(prev, cur)
|
||||
|
||||
Reference in New Issue
Block a user