9 Commits

Author SHA1 Message Date
benvin 7ffa123e3d Merge pull request 'watchpr: only alert on mergeability lost after the baseline' (#20) from benvin/watchpr-baseline-from-first-poll into main
Reviewed-on: #20
2026-09-26 21:26:48 +10:00
unkin-agent 91509cb9b2 docs: name the conflict watchpr will never report
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
2026-09-26 21:14:20 +10:00
unkin-agent 15c527ee56 watchpr: emit the baseline under --json
The baseline is the only notice that a watch started against an already
conflicted PR, and it was missing from the mode automation uses. It goes to
stderr as a JSON record, leaving stdout a single result record.
2026-09-26 21:14:20 +10:00
unkin-agent 27e48ac45e watchpr: debounce conflicts on elapsed time, not poll count
Merge-input movement arms the conflict rule and no longer clears the run of
non-mergeable observations, so an arming poll's own false cannot be half the
evidence and a base branch moving under every poll cannot starve a real
conflict. The run must now span conflictWindow, measured on the tick that
fired each poll.
2026-09-26 21:14:20 +10:00
unkin-agent 7baa194c52 docs: mergeable is a plain bool, not a tri-state on the wire
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
2026-09-26 20:48:31 +10:00
unkin-agent 54c1d868d3 watchpr: arm the conflict rule on a new merge computation
A mergeable=false baseline meant a conflict could never be reported, since
Gitea sends false while it recomputes after a push. Arm on a head or base SHA
change as well as on a mergeable poll, break the conflict run on unknown and
failed polls, and print the baseline with the conditions it suppresses.
2026-09-26 20:48:31 +10:00
unkin-agent 6988be9ff3 watchpr: only alert on mergeability lost after the baseline
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
Track mergeability across the whole run and decode it as a tri-state, so a PR already conflicted when watching starts is polled on instead of reported.
2026-09-26 18:45:48 +10:00
benvin 0cf41409f1 Merge pull request 'agentpr: close and reopen issues' (#19) from benvin/agentpr-issue-state into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #19
2026-09-20 23:08:34 +10:00
unkin-agent 9de9dffab1 agentpr: close and reopen issues
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
- add issue close and issue reopen subcommands
- add GetIssue and SetIssueState to the Gitea client
- fail when the issue is already in the requested state
- document the new subcommands in README.md and AGENTS.md
2026-09-20 23:06:15 +10:00
10 changed files with 1215 additions and 79 deletions
+38 -7
View File
@@ -8,10 +8,11 @@ from Vault, so actions are attributed to the agent rather than to whoever runs
the tool. Setting `AGENT_LOGIN` selects a different agent identity, so a service
like repospawner can run these tools as itself.
- **`agentpr`** — create and edit pull requests and issues, and post comments on
either, as `unkin-agent` (fixes the "tea posts as Ben" attribution problem).
Subcommands: `pr create`, `pr comment`, `pr edit`, `issue create`,
`issue comment`, `issue edit`, `whoami`.
- **`agentpr`** — create and edit pull requests and issues, close and reopen
issues, and post comments on either, as `unkin-agent` (fixes the "tea posts
as Ben" attribution problem). Subcommands: `pr create`, `pr comment`,
`pr edit`, `issue create`, `issue comment`, `issue edit`, `issue close`,
`issue reopen`, `whoami`.
- **`watchpr`** — poll one or more PRs and exit when a tracked PR changes
meaningfully: it merges/closes, gets a new non-agent comment, its CI fails,
or it loses mergeability. Benign transitions (CI pending→success, the agent's
@@ -29,7 +30,7 @@ parsing, watch-state comparison, git worktree helpers).
## Structure
```
cmd/agentpr/main.go # agentpr CLI (pr + issue create/comment/edit, whoami)
cmd/agentpr/main.go # agentpr CLI (pr + issue create/comment/edit, issue close/reopen, whoami)
cmd/watchpr/main.go # watchpr CLI (poll + meaningful-change exit)
cmd/agentws/main.go # agentws CLI (new / list / rm / clean / token / credential)
cmd/agentws/prune.go # agentws prune (classify worktrees, remove the safe ones)
@@ -37,7 +38,7 @@ cmd/agentvault/main.go # agentvault CLI (seed-outpost / seed-oauth)
internal/agent/ # shared plumbing:
token.go # env config + in-process Gitea-token cache
vault.go # AppRole login + read the gitea creds path
gitea.go # Gitea REST client (PR/issue create/edit/get, comments, status, whoami)
gitea.go # Gitea REST client (PR/issue create/edit/get, issue state, comments, status, whoami)
parse.go # owner/repo#N and owner/repo parsing
watch.go # PRState snapshot + MeaningfulChange comparison
git.go # git worktree/clone/fetch helpers (os/exec, no go-git)
@@ -131,7 +132,8 @@ make test # go test -v -race ./...
`internal/agent` covers PR-ref parsing, the `MeaningfulChange` table (benign vs
alerting transitions), request-body construction, and the Vault+Gitea client
against `httptest` servers (fake AppRole login + gitea creds + PR/issue create
+ edit / comment / whoami / status). No live Vault/Gitea access is required for tests.
+ edit / close / reopen / comment / whoami / status). No live Vault/Gitea access
is required for tests.
## agentvault seed-outpost
@@ -218,8 +220,37 @@ wrapped per stage (login / read denied / write denied) via `ErrVaultDenied`.
half-finished (`rebase-merge`, `MERGE_HEAD`, `CHERRY_PICK_HEAD`, …). A directory
whose backing repo is gone is deleted outright, but only ever inside the
worktree root.
- `watchpr` alerts on changes, not on conditions: the baseline snapshot is a real
read, so a PR that is already conflicted or already CI-failing when watching
starts is polled on rather than reported. That suppression is printed with the
baseline, since a silent watcher and a watcher with nothing to say look alike.
The baseline goes to stderr in both modes — a JSON record under `--json` —
because automation is what most needs it and stdout carries the result.
- Mergeability is the one `watchpr` rule needing a run of observations
(`prWatch`). Gitea 1.26 sends `mergeable` as a plain bool — always present,
never null — and sends `false` both for a real conflict and while it recomputes
the merge base after a push, which is exactly when an agent starts a watch. So
`false` counts only once this watch has seen a merge computation start (the PR
was mergeable, or `head.sha`/`base.sha` moved), and then only once the run of
`false`s has spanned `conflictWindow` (2m); a mergeable poll, an unknown one or
a failed poll all break the run. The debounce is wall-clock, measured from the
tick that fired each poll, because what it outlasts is the recompute and
`--interval` spans seconds to hours. Arming and the run are independent:
movement arms and nothing else, since `base.sha` is the base branch tip as of
the response, so it moves for every open PR whenever main does and a run reset
there could never complete on a busy repo. `Mergeability` stays tri-state for
what the bool cannot carry: an absent/null flag from some other Gitea, and a
state no successful poll ever filled in. Head and base SHAs are read only to
arm — a push never alerts. The residue: a conflict landed by the push just
before the watch began, on a head and base that never move again, is never
reported; Gitea's payload has no field separating it from a check in flight
(`merge_base` is the true merge base and does not move on recheck).
- CI "combined status" comes from `/commits/{sha}/status`; an empty head SHA
yields an empty state without an API call.
- Gitea backs every PR with an issue of the same number and serves comments from
`/issues/{n}/comments`, so `agentpr pr comment` and `agentpr issue comment`
are one implementation under two flag names (`--pr` / `--issue`).
- `issue close`/`issue reopen` read the issue before the PATCH: Gitea answers a
no-op state change with 200, so without the read an already-closed issue would
report success. There is no `pr close`: closing a pull request is a human's
call, not an agent's.
+26 -3
View File
@@ -6,8 +6,8 @@ token from Vault, so automated PRs, comments and pushes are attributed to the
agent — not to whoever happens to run the command. Set `AGENT_LOGIN` to act as a
different agent identity.
- **`agentpr`** — create and edit pull requests and issues, and post comments on
either, as the agent user.
- **`agentpr`** — create and edit pull requests and issues, close and reopen
issues, and post comments on either, as the agent user.
- **`watchpr`** — poll one or more PRs and exit when one changes in a way worth
acting on.
- **`agentws`** — manage per-branch git worktrees for `unkin-agent`, cloning
@@ -68,6 +68,12 @@ agentpr issue comment --repo unkin/argocd-apps --issue 43 --body "Fixed in #44."
agentpr issue edit --repo unkin/argocd-apps --issue 43 --body "The pipeline fails with ..."
# prints: #<number> <html_url>
# Close or reopen an issue; an issue already in that state is an error, not a
# silent success
agentpr issue close --repo unkin/argocd-apps --issue 43
agentpr issue reopen --repo unkin/argocd-apps --issue 43
# prints: #<number> <state> <html_url>
agentpr --version
agentpr --help
```
@@ -79,7 +85,24 @@ Non-zero exit on any API error.
Poll PRs and exit (reporting what changed) when a tracked PR **merges/closes**,
gets a **new comment from someone other than the agent**, its **CI fails**
(failure/error), or it **loses mergeability** (a conflict appears). Benign
transitions — CI `pending`→`success`, the agent's own comments — are ignored.
transitions — CI `pending`→`success`, the agent's own comments and pushes — are
ignored, and so is any condition the PR was already in at the baseline: a PR
that is already conflicted or already failing when watching starts keeps being
watched, and the baseline line — written to stderr, as a JSON record under
`--json` — says which conditions it is staying silent about.
Gitea reports `mergeable: false` both for a real conflict and while it
recomputes the merge base after a push, so a conflict is only reported once the
watch has seen a merge computation start (the PR was mergeable, or its head or
base commit moved) **and** the non-mergeable polls have then run unbroken for
two minutes. The debounce is a duration, not a poll count, because what it has
to outlast is Gitea's recompute and `--interval` ranges from seconds to hours.
One case is therefore never reported: a conflict introduced by the push
immediately before the watch started, on a PR whose head and base never move
again. Nothing in Gitea's payload separates that from a merge check still in
flight, so watchpr stays silent about it for as long as it runs — the baseline
line is how you see it.
```bash
# Watch until something meaningful happens (default interval 60s)
+48 -2
View File
@@ -9,6 +9,8 @@
// agentpr issue create --repo owner/repo --title T --body B
// agentpr issue comment --repo owner/repo --issue 12 --body "..."
// agentpr issue edit --repo owner/repo --issue 12 --title T --body B
// agentpr issue close --repo owner/repo --issue 12
// agentpr issue reopen --repo owner/repo --issue 12
// agentpr whoami
package main
@@ -69,9 +71,15 @@ func newPRCmd() *cobra.Command {
func newIssueCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "issue",
Short: "File and edit issues, and post issue comments",
Short: "File, edit, close and reopen issues, and post issue comments",
}
cmd.AddCommand(newIssueCreateCmd(), newCommentCmd("issue", "issue", "Post a comment on an issue"), newIssueEditCmd())
cmd.AddCommand(
newIssueCreateCmd(),
newCommentCmd("issue", "issue", "Post a comment on an issue"),
newIssueEditCmd(),
newIssueStateCmd("close", "Close an issue", agent.IssueStateClosed),
newIssueStateCmd("reopen", "Reopen a closed issue", agent.IssueStateOpen),
)
return cmd
}
@@ -301,6 +309,44 @@ func newIssueEditCmd() *cobra.Command {
return cmd
}
// newIssueStateCmd builds `issue close` and `issue reopen`, which differ only
// in the state they ask for. An issue already in that state is an error, not a
// silent success: Gitea answers the PATCH with 200 either way.
func newIssueStateCmd(use, short, state string) *cobra.Command {
var repo string
var issue int
cmd := &cobra.Command{
Use: use,
Short: short,
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if issue <= 0 {
return fmt.Errorf("--issue must be a positive issue number")
}
c, err := client()
if err != nil {
return err
}
updated, err := c.SetIssueState(owner+"/"+name, issue, state)
if err != nil {
return err
}
fmt.Printf("#%d %s %s\n", updated.Number, updated.State, updated.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&issue, "issue", 0, "Issue number (required)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("issue")
return cmd
}
func newWhoamiCmd() *cobra.Command {
return &cobra.Command{
Use: "whoami",
+53
View File
@@ -149,3 +149,56 @@ func TestCommentCommandsStayInStep(t *testing.T) {
t.Error("issue comment must take --issue and only --issue")
}
}
// Both state commands need a repo and an issue number; cobra and the RunE
// guard must reject their absence before anything reaches for a token.
func TestIssueStateCommandsRequireFlags(t *testing.T) {
for _, verb := range []string{"close", "reopen"} {
t.Run(verb+" without --issue", func(t *testing.T) {
err := execute("issue", verb, "--repo", "unkin/repo")
if err == nil {
t.Fatal("Execute() = nil, want an error when --issue is missing")
}
if !strings.Contains(err.Error(), "issue") {
t.Errorf("error = %q, want it to name the missing --issue flag", err)
}
})
t.Run(verb+" without --repo", func(t *testing.T) {
err := execute("issue", verb, "--issue", "12")
if err == nil {
t.Fatal("Execute() = nil, want an error when --repo is missing")
}
if !strings.Contains(err.Error(), "repo") {
t.Errorf("error = %q, want it to name the missing --repo flag", err)
}
})
t.Run(verb+" with a zero --issue", func(t *testing.T) {
err := execute("issue", verb, "--repo", "unkin/repo", "--issue", "0")
if err == nil {
t.Fatal("Execute() = nil, want an error for a non-positive issue number")
}
if !strings.Contains(err.Error(), "--issue must be a positive") {
t.Errorf("error = %q, want it to reject the issue number", err)
}
})
t.Run(verb+" with a malformed --repo", func(t *testing.T) {
if err := execute("issue", verb, "--repo", "not-a-repo", "--issue", "12"); err == nil {
t.Fatal("Execute() = nil, want error for a malformed --repo")
}
})
}
}
// Closing and reopening are issue-only: a PR is closed by a human, so the pr
// group must not grow these verbs by accident.
func TestPRHasNoStateCommands(t *testing.T) {
for _, verb := range []string{"close", "reopen"} {
if cmd, _, err := newRootCmd().Find([]string{"pr", verb}); err == nil && cmd.Name() == verb {
t.Errorf("pr %s exists; closing a PR is not agentpr's to do", verb)
}
cmd, _, err := newRootCmd().Find([]string{"issue", verb})
if err != nil || cmd.Name() != verb {
t.Fatalf("issue %s not found: %v", verb, err)
}
}
}
+71 -9
View File
@@ -1,8 +1,15 @@
// Command watchpr polls one or more Gitea pull requests and exits when a
// tracked PR changes in a way worth alerting on: it merges or closes, gets a
// new comment from someone other than the agent, its CI fails, or it loses
// mergeability. Benign transitions (CI pending→success, the agent's own
// comments) are ignored.
// 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 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
@@ -13,7 +20,9 @@ package main
import (
"encoding/json"
"fmt"
"io"
"os"
"strings"
"time"
"git.unkin.net/unkin/agent-tools/internal/agent"
@@ -43,7 +52,8 @@ func newRootCmd() *cobra.Command {
Short: "Poll Gitea PRs and exit when one changes meaningfully.",
Long: "watchpr polls each PR every --interval and exits (reporting what changed)\n" +
"when a PR merges/closes, gets a new non-agent comment, its CI fails, or it\n" +
"loses mergeability. Accepts refs as owner/repo#N or owner/repo:N.",
"loses mergeability after the baseline. Conditions already true at the\n" +
"baseline are printed, not alerted on. Refs take owner/repo#N or owner/repo:N.",
Version: version,
Args: cobra.ArbitraryArgs,
SilenceUsage: true,
@@ -126,10 +136,8 @@ func runWatch(c *agent.GiteaClient, refs []agent.PRRef, interval time.Duration,
ticker := time.NewTicker(interval)
defer ticker.Stop()
onBaseline := func() {
if !jsonMode {
fmt.Fprintf(os.Stderr, "watching %d PR(s) every %s; baseline established\n", len(refs), interval)
}
onBaseline := func(states []agent.PRState) {
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)
@@ -143,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
@@ -176,8 +210,36 @@ func report(key, reason string, st agent.PRState, jsonMode bool) {
}
func printState(st agent.PRState) {
fmt.Printf("%s state=%s merged=%t mergeable=%t ci=%s head=%s comments(non-agent)=%d\n",
st.Ref.String(), st.State, st.Merged, st.Mergeable, ciOrNone(st.CIStatus), shortSHA(st.HeadSHA), st.NonAgentComments)
fmt.Println(stateLine(st))
}
func stateLine(st agent.PRState) string {
return fmt.Sprintf("%s state=%s merged=%t mergeable=%s ci=%s head=%s base=%s comments(non-agent)=%d",
st.Ref.String(), st.State, st.Merged, st.Mergeable, ciOrNone(st.CIStatus),
shortSHA(st.HeadSHA), shortSHA(st.BaseSHA), st.NonAgentComments)
}
// baselineLine describes the state a watch started from, naming the conditions
// it will deliberately stay silent about: watchpr alerts on changes, so a PR
// that is already conflicted or already CI-red produces no alert for either, and
// that suppression has to be visible to whoever started the watch.
func baselineLine(st agent.PRState) string {
line := " baseline " + stateLine(st)
if s := suppressedAtBaseline(st); s != "" {
return line + " -- " + s + "; not alerting on a pre-existing condition"
}
return line
}
func suppressedAtBaseline(st agent.PRState) string {
var conds []string
if st.Mergeable == agent.MergeNo {
conds = append(conds, "already non-mergeable (a real conflict, or Gitea still recomputing)")
}
if st.CIStatus == "failure" || st.CIStatus == "error" {
conds = append(conds, "CI already "+st.CIStatus)
}
return strings.Join(conds, ", ")
}
func ciOrNone(s string) string {
+74
View File
@@ -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"
)
@@ -182,3 +185,74 @@ func TestDescribeFailureNamesTheCause(t *testing.T) {
})
}
}
// A watch started against an already conflicted or CI-red PR sends no alert for
// either, so the baseline line has to say that is what it is doing: before this,
// such a run printed nothing in either direction.
func TestBaselineLineNamesSuppressedConditions(t *testing.T) {
st := agent.PRState{
Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7},
State: "open",
Mergeable: agent.MergeNo,
CIStatus: "failure",
HeadSHA: "cafebabecafebabe",
BaseSHA: "deadbeefdeadbeef",
}
got := baselineLine(st)
for _, want := range []string{
"unkin/repo#7", "mergeable=false", "ci=failure", "head=cafebabe", "base=deadbeef",
"already non-mergeable", "CI already failure", "not alerting",
} {
if !strings.Contains(got, want) {
t.Errorf("baselineLine = %q, want it to mention %q", got, want)
}
}
clean := st
clean.Mergeable = agent.MergeYes
clean.CIStatus = "success"
if got := baselineLine(clean); strings.Contains(got, "not alerting") {
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())
}
}
+141
View File
@@ -946,3 +946,144 @@ func TestEditIssueSendsOnlySuppliedFields(t *testing.T) {
})
}
}
// Closing and reopening differ only in the state sent; both must read the
// issue first and then PATCH the issue endpoint with that state alone.
func TestSetIssueStateRequest(t *testing.T) {
tests := []struct {
name string
current string
state string
}{
{"close an open issue", IssueStateOpen, IssueStateClosed},
{"reopen a closed issue", IssueStateClosed, IssueStateOpen},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var methods []string
var gotBody map[string]any
var gotPath string
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/12", func(w http.ResponseWriter, r *http.Request) {
methods = append(methods, r.Method)
gotPath = r.URL.Path
if r.Method == http.MethodGet {
_, _ = io.WriteString(w, `{"number":12,"state":"`+tt.current+`","title":"T","html_url":"https://git.unkin.net/unkin/repo/issues/12"}`)
return
}
_ = json.NewDecoder(r.Body).Decode(&gotBody)
_, _ = io.WriteString(w, `{"number":12,"state":"`+tt.state+`","title":"T","html_url":"https://git.unkin.net/unkin/repo/issues/12"}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "gitea-abc", HTTP: srv.Client()}
issue, err := c.SetIssueState("unkin/repo", 12, tt.state)
if err != nil {
t.Fatalf("SetIssueState: %v", err)
}
if len(methods) != 2 || methods[0] != http.MethodGet || methods[1] != http.MethodPatch {
t.Errorf("requests = %v, want a GET then a PATCH", methods)
}
if gotPath != "/api/v1/repos/unkin/repo/issues/12" {
t.Errorf("path = %q", gotPath)
}
if len(gotBody) != 1 || gotBody["state"] != tt.state {
t.Errorf("payload = %v, want only {\"state\":%q}", gotBody, tt.state)
}
if issue.State != tt.state || issue.Number != 12 {
t.Errorf("parsed issue = %+v", issue)
}
})
}
}
// Gitea answers a no-op state change with 200, so an issue already in the
// requested state must fail rather than report a change that never happened —
// and no PATCH may be sent.
func TestSetIssueStateAlreadyInState(t *testing.T) {
patches := 0
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/12", func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodPatch {
patches++
}
_, _ = io.WriteString(w, `{"number":12,"state":"closed","title":"T","html_url":"https://git.unkin.net/unkin/repo/issues/12"}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
_, err := c.SetIssueState("unkin/repo", 12, IssueStateClosed)
if err == nil {
t.Fatal("expected an error closing an already-closed issue")
}
if !errors.Is(err, ErrIssueStateUnchanged) {
t.Errorf("errors.Is(%v, ErrIssueStateUnchanged) = false", err)
}
if !strings.Contains(err.Error(), "unkin/repo#12") {
t.Errorf("error %q should name the issue", err)
}
if patches != 0 {
t.Errorf("PATCH requests = %d, want 0", patches)
}
}
// A non-2xx on either leg must surface the API's own message, not a bare
// status, and must not be mistaken for a successful change.
func TestSetIssueStateAPIError(t *testing.T) {
tests := []struct {
name string
failOn string
status int
}{
{"read fails", http.MethodGet, http.StatusNotFound},
{"write fails", http.MethodPatch, http.StatusForbidden},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/12", func(w http.ResponseWriter, r *http.Request) {
if r.Method == tt.failOn {
w.WriteHeader(tt.status)
_, _ = io.WriteString(w, `{"message":"no dice","url":"https://git.unkin.net/api/swagger","errors":null}`)
return
}
_, _ = io.WriteString(w, `{"number":12,"state":"open","title":"T","html_url":"https://git.unkin.net/unkin/repo/issues/12"}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
_, err := c.SetIssueState("unkin/repo", 12, IssueStateClosed)
if err == nil {
t.Fatalf("expected an error when %s returns %d", tt.failOn, tt.status)
}
if !strings.Contains(err.Error(), `"message":"no dice"`) {
t.Errorf("error %q should carry the API message", err)
}
})
}
}
// Only Gitea's two states are accepted, and a bad one is rejected before any
// request goes out.
func TestSetIssueStateRejectsUnknownState(t *testing.T) {
requests := 0
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requests++
}))
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
_, err := c.SetIssueState("unkin/repo", 12, "merged")
if err == nil {
t.Fatal("expected an error for an unknown state")
}
if !strings.Contains(err.Error(), `invalid issue state "merged"`) {
t.Errorf("error = %q, want it to name the invalid state", err)
}
if requests != 0 {
t.Errorf("requests = %d, want 0", requests)
}
}
+51 -6
View File
@@ -189,17 +189,22 @@ func (c *GiteaClient) Whoami() (User, error) {
// PullRequest is the subset of Gitea's PR object we track.
type PullRequest struct {
Number int `json:"number"`
State string `json:"state"`
Title string `json:"title"`
Merged bool `json:"merged"`
Mergeable bool `json:"mergeable"`
HTMLURL string `json:"html_url"`
Number int `json:"number"`
State string `json:"state"`
Title string `json:"title"`
Merged bool `json:"merged"`
Mergeable Mergeability `json:"mergeable"`
HTMLURL string `json:"html_url"`
Head struct {
Sha string `json:"sha"`
Ref string `json:"ref"`
Label string `json:"label"`
} `json:"head"`
// Base.Sha is the base branch tip as of this response, not the merge base
// recorded when the PR was opened.
Base struct {
Sha string `json:"sha"`
} `json:"base"`
}
// prPageSize is the per-page limit for the pulls listing; maxPRPages caps how
@@ -329,6 +334,46 @@ func (c *GiteaClient) EditIssue(repoPath string, number int, opts EditOptions) (
return issue, err
}
// Issue states Gitea accepts on a state change. Gitea has no third state: an
// issue is open or closed.
const (
IssueStateOpen = "open"
IssueStateClosed = "closed"
)
// ErrIssueStateUnchanged reports a state change asked for the state the issue
// is already in. Gitea answers such a PATCH with 200 and changes nothing, so
// without this check closing an already-closed issue would look like it worked.
var ErrIssueStateUnchanged = errors.New("issue is already in that state")
// GetIssue fetches a single issue
// (GET /api/v1/repos/{owner}/{repo}/issues/{index}).
func (c *GiteaClient) GetIssue(repoPath string, number int) (Issue, error) {
var issue Issue
err := c.do(http.MethodGet, fmt.Sprintf("/api/v1/repos/%s/issues/%d", repoPath, number), nil, &issue)
return issue, err
}
// SetIssueState closes or reopens an issue. It reads the issue first so an
// issue already in the requested state fails with ErrIssueStateUnchanged
// instead of reporting a change that never happened.
func (c *GiteaClient) SetIssueState(repoPath string, number int, state string) (Issue, error) {
if state != IssueStateOpen && state != IssueStateClosed {
return Issue{}, fmt.Errorf("invalid issue state %q: want %q or %q", state, IssueStateOpen, IssueStateClosed)
}
current, err := c.GetIssue(repoPath, number)
if err != nil {
return Issue{}, err
}
if current.State == state {
return current, fmt.Errorf("%s#%d: %w (%s)", repoPath, number, ErrIssueStateUnchanged, state)
}
var issue Issue
payload := map[string]string{"state": state}
err = c.do(http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/issues/%d", repoPath, number), payload, &issue)
return issue, err
}
// Comment is the subset of an issue comment we track.
type Comment struct {
ID int64 `json:"id"`
+159 -28
View File
@@ -3,6 +3,7 @@ package agent
import (
"errors"
"fmt"
"strings"
"time"
)
@@ -16,17 +17,65 @@ func IsPRGone(err error) bool {
return errors.Is(err, errPRGone)
}
// Mergeability is Gitea's mergeable flag. Gitea 1.26 always sends a plain bool,
// and sends false both for a real conflict and while it recomputes the merge
// base after a push, so false on its own decides nothing (prWatch resolves it).
// Unknown covers what the bool cannot carry: an absent or null flag from another
// Gitea build, and a snapshot no successful poll ever filled in.
type Mergeability int
const (
MergeUnknown Mergeability = iota
MergeYes
MergeNo
)
func (m Mergeability) String() string {
switch m {
case MergeYes:
return "true"
case MergeNo:
return "false"
}
return "unknown"
}
func (m Mergeability) MarshalJSON() ([]byte, error) {
switch m {
case MergeYes:
return []byte("true"), nil
case MergeNo:
return []byte("false"), nil
}
return []byte("null"), nil
}
func (m *Mergeability) UnmarshalJSON(b []byte) error {
switch strings.TrimSpace(string(b)) {
case "true":
*m = MergeYes
case "false":
*m = MergeNo
case "null":
*m = MergeUnknown
default:
return fmt.Errorf("mergeable: unexpected value %s", b)
}
return nil
}
// PRState is a point-in-time snapshot of the PR attributes watchpr tracks.
type PRState struct {
Ref PRRef `json:"ref"`
State string `json:"state"` // open / closed
Merged bool `json:"merged"`
HeadSHA string `json:"head_sha"`
Mergeable bool `json:"mergeable"`
CIStatus string `json:"ci_status"` // success / pending / failure / error / ""
NonAgentComments int `json:"non_agent_comments"`
Title string `json:"title"`
URL string `json:"url"`
Ref PRRef `json:"ref"`
State string `json:"state"` // open / closed
Merged bool `json:"merged"`
HeadSHA string `json:"head_sha"`
BaseSHA string `json:"base_sha"`
Mergeable Mergeability `json:"mergeable"`
CIStatus string `json:"ci_status"` // success / pending / failure / error / ""
NonAgentComments int `json:"non_agent_comments"`
Title string `json:"title"`
URL string `json:"url"`
}
// FetchState builds a PRState for the given ref. agentLogin's comments are
@@ -55,6 +104,7 @@ func FetchState(c *GiteaClient, ref PRRef, agentLogin string) (PRState, error) {
State: pr.State,
Merged: pr.Merged,
HeadSHA: pr.Head.Sha,
BaseSHA: pr.Base.Sha,
Mergeable: pr.Mergeable,
CIStatus: ci,
NonAgentComments: countNonAgentComments(comments, agentLogin),
@@ -95,6 +145,90 @@ func terminalState(st PRState) (bool, string) {
return false, ""
}
// conflictWindow is how long an unbroken run of non-mergeable polls must span
// before it is reported as a conflict. What the run has to outlast is Gitea's
// merge recompute, which is a duration, so counting polls cannot express it:
// --interval varies from seconds to hours between callers. Recomputes have been
// observed finishing in ~5-20s with no bound on the tail, so this is set well
// clear of that; the cost of it being too long is only a later conflict alert,
// while too short is a false one.
const conflictWindow = 2 * time.Minute
// prWatch tracks one PR across polls, because mergeability needs more memory
// than the previous snapshot. Gitea reports mergeable=false while it recomputes
// the merge base after a push, so a false is only trusted once this watch has
// seen a merge computation start: the PR was mergeable at some point, or its
// head or base SHA moved. A bare false inherited from the baseline says nothing
// -- it is equally a conflict the operator is already waiting on and a recompute
// in flight -- so it arms nothing. Arming and the run of falses are independent:
// movement only arms, because a base branch that moves under the PR on every
// push to main would otherwise restart the run forever.
type prWatch struct {
prev PRState
armed bool
// conflictSince is when the current unbroken run of non-mergeable polls
// began; zero when no run is in progress.
conflictSince time.Time
}
func newPRWatch(baseline PRState) *prWatch {
return &prWatch{prev: baseline, armed: baseline.Mergeable == MergeYes}
}
// mergeInputsChanged reports whether the commits Gitea merges have moved, which
// starts a fresh merge computation whose result is attributable to this watch.
func mergeInputsChanged(prev, cur PRState) bool {
return cur.HeadSHA != prev.HeadSHA || cur.BaseSHA != prev.BaseSHA
}
// track folds one snapshot's mergeability into the run of observations. The
// observations that confirm a conflict must be adjacent, so anything but
// another non-mergeable one breaks the run.
func (w *prWatch) track(st PRState, now time.Time) {
switch st.Mergeable {
case MergeNo:
if w.conflictSince.IsZero() {
w.conflictSince = now
}
case MergeYes:
w.armed = true
w.conflictSince = time.Time{}
default:
w.conflictSince = time.Time{}
}
}
// missed records a poll that never produced a snapshot; the run of adjacent
// non-mergeable observations does not survive the gap.
func (w *prWatch) missed() {
w.conflictSince = time.Time{}
}
// confirmed reports whether the run of non-mergeable observations has spanned
// the recompute window. The run starts at its first observation, so a lone
// non-mergeable poll never confirms anything whatever the interval.
func (w *prWatch) confirmed(now time.Time) bool {
return !w.conflictSince.IsZero() && now.Sub(w.conflictSince) >= conflictWindow
}
// observe folds in the newest snapshot, taken at now, and reports whether the
// watch should end.
func (w *prWatch) observe(cur PRState, now time.Time) (bool, string) {
changed, reason := MeaningfulChange(w.prev, cur)
if mergeInputsChanged(w.prev, cur) {
w.armed = true
}
w.prev = cur
w.track(cur, now)
if changed {
return true, reason
}
if w.armed && w.confirmed(now) && cur.State == "open" {
return true, "PR lost mergeability (conflict)"
}
return false, ""
}
// MaxPollFailures is how many consecutive failed polls of the same PR are
// tolerated before Watch gives up. The abort fires on the 20th failed tick, so
// at watchpr's default 60s interval a watch rides out ~19 minutes of failure.
@@ -109,10 +243,11 @@ const MaxPollFailures = 20
// is gone, renamed, or no longer visible), and MaxPollFailures consecutive
// failures of one PR all abort, because a watcher that sees nothing must not
// look healthy.
// onBaseline, if set, fires once after all baselines are captured and before the
// first tick.
func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Time, onBaseline func(), onError func(PRRef, error)) (WatchResult, error) {
prev := make(map[string]PRState, len(refs))
// onBaseline, if set, receives every captured baseline once, before the first
// tick, so a caller can show what state the watch started from.
func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Time, onBaseline func([]PRState), onError func(PRRef, error)) (WatchResult, error) {
watches := make(map[string]*prWatch, len(refs))
baselines := make([]PRState, 0, len(refs))
for _, ref := range refs {
st, err := f.FetchState(ref, agentLogin)
if err != nil {
@@ -121,13 +256,16 @@ func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Ti
if terminal, reason := terminalState(st); terminal {
return WatchResult{Ref: ref, Reason: reason, State: st}, nil
}
prev[ref.String()] = st
watches[ref.String()] = newPRWatch(st)
baselines = append(baselines, st)
}
if onBaseline != nil {
onBaseline()
onBaseline(baselines)
}
fails := make(map[string]int, len(refs))
for range ticks {
// The tick carries the time it fired, which is the clock the conflict
// window is measured on.
for now := range ticks {
for _, ref := range refs {
key := ref.String()
cur, err := f.FetchState(ref, agentLogin)
@@ -136,6 +274,7 @@ func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Ti
return WatchResult{}, fmt.Errorf("polling %s: %w", key, err)
}
fails[key]++
watches[key].missed()
if onError != nil {
onError(ref, err)
}
@@ -145,10 +284,9 @@ func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Ti
continue
}
fails[key] = 0
if changed, reason := MeaningfulChange(prev[key], cur); changed {
if changed, reason := watches[key].observe(cur, now); changed {
return WatchResult{Ref: ref, Reason: reason, State: cur}, nil
}
prev[key] = cur
}
}
return WatchResult{}, nil
@@ -172,15 +310,15 @@ func isFailedCI(state string) bool {
// MeaningfulChange compares a previous state to the current one and reports
// whether a change warrants alerting the operator, with a human-readable
// reason. Benign transitions (CI pending→success, the agent's own comments, an
// unchanged snapshot) return false.
// reason. Benign transitions (CI pending→success, the agent's own comments, a
// new head or base commit, an unchanged snapshot) return false. Mergeability is
// not decided here: it takes a whole run of observations, which prWatch keeps.
//
// Alerting conditions:
// - the PR merged
// - 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) for two consecutive polls
func MeaningfulChange(prev, cur PRState) (bool, string) {
if !prev.Merged && cur.Merged {
return true, "PR merged"
@@ -195,12 +333,5 @@ func MeaningfulChange(prev, cur PRState) (bool, string) {
if isFailedCI(cur.CIStatus) && !isFailedCI(prev.CIStatus) {
return true, "CI failed (" + cur.CIStatus + ")"
}
// 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, ""
}
+554 -24
View File
@@ -1,6 +1,7 @@
package agent
import (
"encoding/json"
"errors"
"fmt"
"net/http"
@@ -17,7 +18,8 @@ func base() PRState {
State: "open",
Merged: false,
HeadSHA: "abc123",
Mergeable: true,
BaseSHA: "base000",
Mergeable: MergeYes,
CIStatus: "pending",
NonAgentComments: 0,
}
@@ -66,25 +68,11 @@ func TestMeaningfulChange(t *testing.T) {
wantChange: true,
},
{
// 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) {},
// Mergeability takes a run of observations, so no pair of snapshots
// decides it here; prWatch owns that rule.
name: "mergeable false pair alone is not a pairwise change",
mutatePrev: func(s *PRState) { s.Mergeable = MergeNo },
mutate: func(s *PRState) { s.Mergeable = MergeNo },
wantChange: false,
},
{
@@ -222,16 +210,16 @@ func TestWatchDetectsMergeAfterBaseline(t *testing.T) {
merged.Merged = true
f := &fakeFetcher{states: []PRState{open, merged}} // baseline open, then merged
baselineFired := false
var baselines []PRState
ticks := make(chan time.Time, 1)
ticks <- time.Now()
res, err := Watch(f, []PRRef{open.Ref}, "unkin-agent",
ticks, func() { baselineFired = true }, nil)
ticks, func(sts []PRState) { baselines = sts }, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if !baselineFired {
t.Errorf("onBaseline should fire for an open baseline")
if len(baselines) != 1 || baselines[0].Ref != open.Ref {
t.Errorf("onBaseline received %v, want the one open baseline", baselines)
}
if res.Reason != "PR merged" {
t.Errorf("reason = %q, want %q", res.Reason, "PR merged")
@@ -801,3 +789,545 @@ func TestWatchAnonymousKeepsPolling(t *testing.T) {
t.Errorf("fetch calls = %d, want 3 (baseline + two polls)", f.calls)
}
}
// drainableTicks returns a channel holding n ticks and already closed, so Watch
// polls exactly n times and then returns instead of blocking. The ticks are
// spaced far past conflictWindow, so a run of non-mergeable polls confirms on
// its second observation; spacedTicks drives the intervals where it must not.
func drainableTicks(n int) <-chan time.Time {
return spacedTicks(n, 10*time.Minute)
}
// spacedTicks is drainableTicks with the poll interval named. The tick carries
// the time Watch measures the conflict window on, which makes every wall-clock
// assertion in these tests exact and instant.
func spacedTicks(n int, interval time.Duration) <-chan time.Time {
start := time.Date(2026, 9, 24, 12, 0, 0, 0, time.UTC)
ticks := make(chan time.Time, n)
for i := 1; i <= n; i++ {
ticks <- start.Add(time.Duration(i) * interval)
}
close(ticks)
return ticks
}
// The production bug: a PR that was already conflicted (and already CI-failing)
// when watching began must not be reported as having just changed. That state is
// what the watcher is waiting to see resolved, so the loop keeps polling.
func TestWatchIgnoresBaselineConflictAndFailure(t *testing.T) {
stuck := base()
stuck.Mergeable = MergeNo
stuck.CIStatus = "failure"
f := &fakeFetcher{states: []PRState{stuck}}
res, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", drainableTicks(5), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "" {
t.Fatalf("Watch ended with %q; a conflict/failure predating the watch is not a change", res.Reason)
}
if f.calls != 6 {
t.Errorf("fetch calls = %d, want 6 (baseline + 5 polls)", f.calls)
}
}
// Mergeability lost after the baseline still alerts, on the second consecutive
// conflicted poll.
func TestWatchDetectsConflictAfterBaseline(t *testing.T) {
ok := base()
conflicted := base()
conflicted.Mergeable = MergeNo
f := &fakeFetcher{states: []PRState{ok, conflicted, conflicted}}
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "PR lost mergeability (conflict)" {
t.Errorf("reason = %q, want the mergeability loss", res.Reason)
}
if f.calls != 3 {
t.Errorf("fetch calls = %d, want 3 (baseline + the two conflicted polls)", f.calls)
}
}
// CI that goes green→red during the watch alerts.
func TestWatchDetectsCIFailureAfterBaseline(t *testing.T) {
ok := base()
ok.CIStatus = "pending"
green := base()
green.CIStatus = "success"
red := base()
red.CIStatus = "failure"
f := &fakeFetcher{states: []PRState{ok, green, red}}
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "CI failed (failure)" {
t.Errorf("reason = %q, want the CI failure (pending→success must pass silently)", res.Reason)
}
if f.calls != 3 {
t.Errorf("fetch calls = %d, want 3 (the success poll must not end the watch)", f.calls)
}
}
// The agent's own pushes and comments must not end a watch; a comment from
// anyone else must.
func TestWatchIgnoresAgentActivity(t *testing.T) {
start := base()
pushed := base()
pushed.HeadSHA = "def456" // the agent pushed a fix; non-agent comments unchanged
commented := pushed
commented.NonAgentComments = 1
f := &fakeFetcher{states: []PRState{start, pushed, pushed, commented}}
res, err := Watch(f, []PRRef{start.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "new comment from a non-agent user" {
t.Errorf("reason = %q, want the non-agent comment", res.Reason)
}
if f.calls != 4 {
t.Errorf("fetch calls = %d, want 4 (the agent's push and comment must not end the watch)", f.calls)
}
}
func TestPRWatchMergeability(t *testing.T) {
snap := func(m Mergeability) PRState {
s := base()
s.Mergeable = m
return s
}
tests := []struct {
name string
baseline Mergeability
polls []Mergeability
wantPoll int // 1-based poll that ends the watch; 0 for none
}{
{"conflicted before the watch never alerts", MergeNo, []Mergeability{MergeNo, MergeNo, MergeNo}, 0},
{"loss after a mergeable baseline alerts on the second poll", MergeYes, []Mergeability{MergeNo, MergeNo}, 2},
{"a lone conflicted poll is debounced", MergeYes, []Mergeability{MergeNo, MergeYes, MergeNo}, 0},
{"unknown at baseline does not arm the rule", MergeUnknown, []Mergeability{MergeNo, MergeNo, MergeNo}, 0},
{"unknown at baseline then a real loss alerts", MergeUnknown, []Mergeability{MergeYes, MergeNo, MergeNo}, 3},
{"a baseline conflict resolved then lost again alerts", MergeNo, []Mergeability{MergeYes, MergeNo, MergeNo}, 3},
{"unknown between conflicted polls breaks the run", MergeYes, []Mergeability{MergeNo, MergeUnknown, MergeNo}, 0},
{"the run restarts after an unknown", MergeYes, []Mergeability{MergeNo, MergeUnknown, MergeNo, MergeNo}, 4},
{"unknown alone is never a conflict", MergeYes, []Mergeability{MergeUnknown, MergeUnknown, MergeUnknown}, 0},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
w := newPRWatch(snap(tt.baseline))
start := time.Date(2026, 9, 24, 12, 0, 0, 0, time.UTC)
got := 0
for i, m := range tt.polls {
changed, reason := w.observe(snap(m), start.Add(time.Duration(i+1)*10*time.Minute))
if !changed {
continue
}
if reason != "PR lost mergeability (conflict)" {
t.Fatalf("poll %d ended the watch with %q, want a mergeability loss", i+1, reason)
}
got = i + 1
break
}
if got != tt.wantPoll {
t.Errorf("alerted on poll %d, want %d", got, tt.wantPoll)
}
})
}
}
// Gitea 1.26 always sends mergeable as a plain bool, so unknown is reserved for
// what that bool cannot carry: an absent or null flag must decode as unknown
// rather than as a conflict, and unknown must encode back as null.
func TestMergeabilityDecoding(t *testing.T) {
tests := map[string]Mergeability{
`{"number":7}`: MergeUnknown,
`{"number":7,"mergeable":null}`: MergeUnknown,
`{"number":7,"mergeable":true}`: MergeYes,
`{"number":7,"mergeable":false}`: MergeNo,
}
for body, want := range tests {
var pr PullRequest
if err := json.Unmarshal([]byte(body), &pr); err != nil {
t.Fatalf("Unmarshal(%s): %v", body, err)
}
if pr.Mergeable != want {
t.Errorf("Unmarshal(%s) mergeable = %v, want %v", body, pr.Mergeable, want)
}
}
out, err := json.Marshal(PRState{Mergeable: MergeUnknown})
if err != nil {
t.Fatalf("Marshal: %v", err)
}
if !strings.Contains(string(out), `"mergeable":null`) {
t.Errorf("unknown mergeability encoded as %s, want null", out)
}
}
// The inverse of the bug this PR fixed: demanding a mergeable observation to arm
// the rule left a PR that was non-mergeable at baseline silent forever, even
// after a push gave Gitea a fresh merge computation to answer for. A head that
// moves during the watch arms the rule, so the conflict the new head keeps
// reporting is attributable to this watch and is reported.
func TestWatchAlertsWhenAPushArmsABaselineConflict(t *testing.T) {
baseline := base()
baseline.Mergeable = MergeNo // still recomputing; the push is not visible yet
pushed := base()
pushed.Mergeable = MergeNo
pushed.HeadSHA = "def456"
f := &fakeFetcher{states: []PRState{baseline, pushed, pushed, pushed}}
res, err := Watch(f, []PRRef{baseline.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "PR lost mergeability (conflict)" {
t.Fatalf("reason = %q, want the mergeability loss; a conflict must not be unreportable because the baseline caught the recompute", res.Reason)
}
if f.calls != 3 {
t.Errorf("fetch calls = %d, want 3 (baseline + the two conflicted polls on the new head)", f.calls)
}
}
// A base-branch move restarts the same merge computation, so a conflict that
// only becomes visible once someone merges into main alerts too.
func TestWatchAlertsWhenBaseMovedIntoAConflict(t *testing.T) {
baseline := base()
baseline.Mergeable = MergeNo
moved := base()
moved.Mergeable = MergeNo
moved.BaseSHA = "base111"
f := &fakeFetcher{states: []PRState{baseline, moved, moved}}
res, err := Watch(f, []PRRef{baseline.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "PR lost mergeability (conflict)" {
t.Errorf("reason = %q, want the mergeability loss", res.Reason)
}
}
// The fix this PR exists for: a conflict predating the watch, on a head and base
// that never move, is the condition the operator is already waiting on and must
// stay silent however long the watch runs.
func TestWatchNeverAlertsOnAStableBaselineConflict(t *testing.T) {
stuck := base()
stuck.Mergeable = MergeNo
f := &fakeFetcher{states: []PRState{stuck}}
res, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", drainableTicks(100), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "" {
t.Fatalf("Watch ended with %q after 100 unchanged polls; a conflict that predates the watch is not a change", res.Reason)
}
}
// Head and base movement arm the conflict rule but are not themselves alerts: a
// push, or a base that moves under the PR, must not end a watch.
func TestWatchArmingMovementDoesNotAlert(t *testing.T) {
start := base()
pushed := base()
pushed.HeadSHA = "def456"
rebased := base()
rebased.HeadSHA = "789abc"
rebased.BaseSHA = "base111"
f := &fakeFetcher{states: []PRState{start, pushed, rebased}}
res, err := Watch(f, []PRRef{start.Ref}, "unkin-agent", drainableTicks(5), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "" {
t.Fatalf("Watch ended with %q; a new head or base is not a change worth alerting on", res.Reason)
}
}
// The polls that confirm a conflict must be adjacent. A failed poll produces no
// snapshot, so the run cannot span it and two non-adjacent falses do not fire.
func TestWatchConflictRunDoesNotSpanAFailedPoll(t *testing.T) {
ok := base()
conflicted := base()
conflicted.Mergeable = MergeNo
f := &fakeFetcher{
states: []PRState{ok, conflicted, conflicted, conflicted, conflicted},
errs: []error{nil, nil, errors.New("HTTP 502"), nil, nil},
}
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", drainableTicks(4), nil, func(PRRef, error) {})
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "PR lost mergeability (conflict)" {
t.Fatalf("reason = %q, want the mergeability loss on the two adjacent polls", res.Reason)
}
if f.calls != 5 {
t.Errorf("fetch calls = %d, want 5: the falses either side of the failed poll are not a run", f.calls)
}
}
// perRefFetcher scripts a separate sequence per ref, so a multi-ref watch can be
// driven with each PR doing something different.
type perRefFetcher struct {
states map[string][]PRState
calls map[string]int
}
func (f *perRefFetcher) FetchState(ref PRRef, _ string) (PRState, error) {
key := ref.String()
seq := f.states[key]
i := f.calls[key]
if i >= len(seq) {
i = len(seq) - 1
}
f.calls[key]++
return seq[i], nil
}
// Each ref keeps its own run of observations: one PR's mergeability, pushes and
// resolutions must neither arm nor disarm another's conflict rule.
func TestWatchTracksRefsIndependently(t *testing.T) {
refA := PRRef{Owner: "unkin", Repo: "repo", Number: 1}
refB := PRRef{Owner: "unkin", Repo: "repo", Number: 2}
snap := func(ref PRRef, m Mergeability, head string) PRState {
s := base()
s.Ref = ref
s.Mergeable = m
s.HeadSHA = head
return s
}
tests := []struct {
name string
a, b []PRState
want string // reason, "" for no alert
}{
{
name: "a mergeable neighbour does not arm a baseline conflict",
a: []PRState{snap(refA, MergeYes, "aaa")},
b: []PRState{snap(refB, MergeNo, "bbb")},
want: "",
},
{
name: "a neighbour's push does not arm a baseline conflict",
a: []PRState{snap(refA, MergeYes, "aaa"), snap(refA, MergeYes, "aa2"), snap(refA, MergeYes, "aa3")},
b: []PRState{snap(refB, MergeNo, "bbb")},
want: "",
},
{
name: "a neighbour's mergeable polls do not clear another's run",
a: []PRState{snap(refA, MergeYes, "aaa")},
b: []PRState{snap(refB, MergeYes, "bbb"), snap(refB, MergeNo, "bbb"), snap(refB, MergeNo, "bbb")},
want: "PR lost mergeability (conflict)",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f := &perRefFetcher{
states: map[string][]PRState{refA.String(): tt.a, refB.String(): tt.b},
calls: map[string]int{},
}
res, err := Watch(f, []PRRef{refA, refB}, "unkin-agent", drainableTicks(6), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != tt.want {
t.Fatalf("reason = %q, want %q", res.Reason, tt.want)
}
if tt.want != "" && res.Ref != refB {
t.Errorf("alert names %s, want %s", res.Ref, refB)
}
})
}
}
// A watch that will deliberately stay silent about a pre-existing conflict has
// to hand its caller the baseline it is staying silent about.
func TestWatchReportsBaselineStates(t *testing.T) {
stuck := base()
stuck.Mergeable = MergeNo
stuck.CIStatus = "failure"
f := &fakeFetcher{states: []PRState{stuck}}
var got []PRState
if _, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", drainableTicks(1),
func(sts []PRState) { got = sts }, nil); err != nil {
t.Fatalf("Watch: %v", err)
}
if len(got) != 1 {
t.Fatalf("onBaseline received %d state(s), want 1", len(got))
}
if got[0].Mergeable != MergeNo || got[0].CIStatus != "failure" {
t.Errorf("baseline = %+v, want the conflicted, CI-red snapshot the watch started from", got[0])
}
}
// The residual gap, pinned so it is a decision rather than an accident: the
// agent pushes, the push conflicts, watchpr starts inside Gitea's recompute and
// nothing moves again. Every poll answers false and none of them is
// attributable to this watch, so no alert is ever sent -- the baseline line is
// the only notice. Gitea's payload carries no field that separates this from a
// merge check still running.
func TestWatchNeverAlertsOnAConflictLandedByThePushBeforeTheWatch(t *testing.T) {
pushed := base()
pushed.Mergeable = MergeNo
pushed.HeadSHA = "def456"
f := &fakeFetcher{states: []PRState{pushed}}
res, err := Watch(f, []PRRef{pushed.Ref}, "unkin-agent", drainableTicks(50), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "" {
t.Fatalf("Watch ended with %q; nothing distinguishes this conflict from a merge check in flight, so it must stay silent", res.Reason)
}
}
// The whole mergeability rule as a sequence table, driven through Watch at the
// intervals that decide it. The debounce is wall-clock, so the same sequence of
// snapshots must alert or stay silent according to how far apart the polls are.
func TestWatchConflictSequences(t *testing.T) {
snap := func(m Mergeability, head, bse string) PRState {
st := base()
st.Mergeable = m
st.HeadSHA = head
st.BaseSHA = bse
return st
}
repeat := func(st PRState, n int) []PRState {
out := make([]PRState, n)
for i := range out {
out[i] = st
}
return out
}
movingBase := func(m Mergeability, n int) []PRState {
out := make([]PRState, n)
for i := range out {
out[i] = snap(m, "h1", fmt.Sprintf("b%d", i))
}
return out
}
const (
quick = 5 * time.Second // well inside conflictWindow
normal = 60 * time.Second // watchpr's default
relaxed = 10 * time.Minute // past conflictWindow in a single gap
)
const conflict = "PR lost mergeability (conflict)"
yes := snap(MergeYes, "h1", "b1")
no := snap(MergeNo, "h1", "b1")
pushedNo := snap(MergeNo, "h2", "b1")
movedNo := snap(MergeNo, "h1", "b2")
tests := []struct {
name string
interval time.Duration
baseline PRState
polls []PRState
errs []error
want string
}{
{
name: "a push then the recompute's falses is not a conflict",
interval: normal, baseline: yes, polls: repeat(pushedNo, 2), want: "",
},
{
name: "a fast poller rides out the whole recompute after a push",
interval: quick, baseline: yes, polls: repeat(pushedNo, 20), want: "",
},
{
name: "a base move then the recompute's falses is not a conflict",
interval: normal, baseline: no, polls: repeat(movedNo, 2), want: "",
},
{
name: "a base moving under every poll still confirms a conflict",
interval: normal, baseline: yes, polls: movingBase(MergeNo, 20), want: conflict,
},
{
name: "a conflict that predates the watch stays silent forever",
interval: relaxed, baseline: no, polls: repeat(no, 50), want: "",
},
{
name: "a sustained loss after a mergeable baseline alerts",
interval: normal, baseline: yes, polls: repeat(no, 3), want: conflict,
},
{
name: "a mergeable poll breaks the run however long it ran",
interval: relaxed, baseline: yes, polls: []PRState{no, yes, no}, want: "",
},
{
name: "an unknown poll breaks the run however long it ran",
interval: relaxed, baseline: yes, polls: []PRState{no, snap(MergeUnknown, "h1", "b1"), no}, want: "",
},
{
name: "a baseline conflict that clears and returns alerts",
interval: relaxed, baseline: no, polls: []PRState{yes, no, no}, want: conflict,
},
{
name: "a failed poll breaks the run",
interval: relaxed, baseline: yes, polls: []PRState{no, no, no},
errs: []error{nil, nil, errors.New("HTTP 502"), nil}, want: "",
},
{
name: "the run restarts after a failed poll and still confirms",
interval: relaxed, baseline: yes, polls: []PRState{no, no, no, no},
errs: []error{nil, nil, errors.New("HTTP 502"), nil, nil}, want: conflict,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f := &fakeFetcher{states: append([]PRState{tt.baseline}, tt.polls...), errs: tt.errs}
res, err := Watch(f, []PRRef{tt.baseline.Ref}, "unkin-agent",
spacedTicks(len(tt.polls), tt.interval), nil, func(PRRef, error) {})
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != tt.want {
t.Fatalf("reason = %q, want %q", res.Reason, tt.want)
}
})
}
}
// Two refs watched together keep separate runs and separate clocks: a PR being
// pushed to must neither delay nor trigger the conflict its neighbour is really
// in, and the alert must name the conflicted one.
func TestWatchConflictIsolatedFromANeighboursPushes(t *testing.T) {
conflicted := PRRef{Owner: "unkin", Repo: "repo", Number: 1}
pushing := PRRef{Owner: "unkin", Repo: "repo", Number: 2}
snap := func(ref PRRef, m Mergeability, head string) PRState {
st := base()
st.Ref = ref
st.Mergeable = m
st.HeadSHA = head
return st
}
f := &perRefFetcher{
states: map[string][]PRState{
conflicted.String(): {snap(conflicted, MergeYes, "a1"), snap(conflicted, MergeNo, "a1")},
pushing.String(): {snap(pushing, MergeYes, "b1"), snap(pushing, MergeYes, "b2"), snap(pushing, MergeYes, "b3")},
},
calls: map[string]int{},
}
res, err := Watch(f, []PRRef{conflicted, pushing}, "unkin-agent", drainableTicks(4), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "PR lost mergeability (conflict)" {
t.Fatalf("reason = %q, want the conflict on %s", res.Reason, conflicted)
}
if res.Ref != conflicted {
t.Errorf("alert names %s, want %s", res.Ref, conflicted)
}
}