diff --git a/cmd/watchpr/main.go b/cmd/watchpr/main.go index 6861e87..2f49166 100644 --- a/cmd/watchpr/main.go +++ b/cmd/watchpr/main.go @@ -3,8 +3,13 @@ // new comment from someone other than the agent, its CI fails, or it loses // mergeability after the baseline. Benign transitions (CI pending→success, the // agent's own pushes and comments) are ignored, and so is any condition that was -// already true at the baseline -- which watchpr prints, so a run started against -// a conflicted or CI-red PR says so. +// already true at the baseline -- which watchpr prints on stderr (as a JSON +// record under --json), so a run started against a conflicted or CI-red PR says +// so. One case is never reported at all: a conflict introduced by the push +// immediately before the watch started, on a PR whose head and base then never +// move, stays silent for the whole run, because nothing in Gitea's payload +// separates it from a merge check still in flight. The baseline line is the +// only notice of it. // // watchpr owner/repo#12 owner/repo:15 // watchpr --once --json owner/repo#12 @@ -15,6 +20,7 @@ package main import ( "encoding/json" "fmt" + "io" "os" "strings" "time" @@ -131,13 +137,7 @@ func runWatch(c *agent.GiteaClient, refs []agent.PRRef, interval time.Duration, defer ticker.Stop() onBaseline := func(states []agent.PRState) { - if jsonMode { - return - } - fmt.Fprintf(os.Stderr, "watching %d PR(s) every %s; baseline established\n", len(refs), interval) - for _, st := range states { - fmt.Fprintln(os.Stderr, baselineLine(st)) - } + emitBaselines(os.Stderr, states, interval, jsonMode) } onError := func(ref agent.PRRef, err error) { fmt.Fprintf(os.Stderr, "warning: polling %s: %v\n", ref.String(), err) @@ -151,6 +151,32 @@ func runWatch(c *agent.GiteaClient, refs []agent.PRRef, interval time.Duration, return nil } +// baselineRecord is the --json form of baselineLine. Both go to stderr, leaving +// the stdout contract a single result record: a caller automating watchpr is +// precisely the one who needs to be told the watch started against a PR that is +// already conflicted, since no alert will ever follow for it. +type baselineRecord struct { + Baseline bool `json:"baseline"` + Suppressed string `json:"suppressed,omitempty"` + State agent.PRState `json:"state"` +} + +// emitBaselines writes the state each watch started from, in whichever form the +// caller asked for. +func emitBaselines(w io.Writer, states []agent.PRState, interval time.Duration, jsonMode bool) { + if jsonMode { + enc := json.NewEncoder(w) + for _, st := range states { + _ = enc.Encode(baselineRecord{Baseline: true, Suppressed: suppressedAtBaseline(st), State: st}) + } + return + } + _, _ = fmt.Fprintf(w, "watching %d PR(s) every %s; baseline established\n", len(states), interval) + for _, st := range states { + _, _ = fmt.Fprintln(w, baselineLine(st)) + } +} + // describeFailure names the cause of a terminal failure so a watcher that stops // says why. An anonymous rejection, a permission boundary and a token that // outlived its Vault lease are three different problems and only the last is diff --git a/cmd/watchpr/main_test.go b/cmd/watchpr/main_test.go index f009fbf..f9d2d25 100644 --- a/cmd/watchpr/main_test.go +++ b/cmd/watchpr/main_test.go @@ -1,6 +1,8 @@ package main import ( + "bytes" + "encoding/json" "errors" "fmt" "io" @@ -8,6 +10,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "git.unkin.net/unkin/agent-tools/internal/agent" ) @@ -212,3 +215,44 @@ func TestBaselineLineNamesSuppressedConditions(t *testing.T) { t.Errorf("baselineLine = %q, want no suppression note for a clean baseline", got) } } + +// --json is the mode automation uses, and automation is exactly who needs to be +// told the watch started against an already-conflicted PR -- the one case that +// will never produce an alert. It must therefore be emitted in JSON mode too, +// on stderr, where it cannot corrupt the result record on stdout. +func TestBaselineIsEmittedInJSONMode(t *testing.T) { + st := agent.PRState{ + Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7}, + State: "open", + Mergeable: agent.MergeNo, + CIStatus: "failure", + HeadSHA: "cafebabecafebabe", + } + var buf bytes.Buffer + emitBaselines(&buf, []agent.PRState{st}, 30*time.Second, true) + + var got baselineRecord + if err := json.Unmarshal(buf.Bytes(), &got); err != nil { + t.Fatalf("baseline is not a JSON record (%v); got %q", err, buf.String()) + } + if !got.Baseline { + t.Error("record does not mark itself as the baseline") + } + if got.State.Ref != st.Ref || got.State.Mergeable != agent.MergeNo { + t.Errorf("record state = %+v, want the conflicted snapshot the watch started from", got.State) + } + for _, want := range []string{"already non-mergeable", "CI already failure"} { + if !strings.Contains(got.Suppressed, want) { + t.Errorf("suppressed = %q, want it to mention %q", got.Suppressed, want) + } + } + + buf.Reset() + clean := st + clean.Mergeable = agent.MergeYes + clean.CIStatus = "success" + emitBaselines(&buf, []agent.PRState{clean}, 30*time.Second, true) + if strings.Contains(buf.String(), "suppressed") { + t.Errorf("clean baseline = %q, want no suppression field", buf.String()) + } +}