12 Commits

Author SHA1 Message Date
benvin 5488a1ddc6 Merge pull request 'watchpr: start the conflict window at the arm' (#21) from benvin/watchpr-arm-window-start into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #21
2026-09-26 23:45:14 +10:00
unkin-agent ebdd25f725 watchpr: describe arming as a one-way latch, not a window restart
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
The docs claimed every later head/base move restarts the conflict window.
arm() returns early once armed, so only the first move sets armedAt and
every move after it is a no-op. State the real trade-off instead: repeated
moves do not extend the debounce, so a conflict can be confirmed while the
newest recompute is younger than the window.

Also narrow the --json stderr claim to the notices watchpr writes itself
(cobra's terminal Error: line is plain text), rename the test to what it
covers, and stop the unknown-mergeability baseline implying a false answer
arms the rule.
2026-09-26 22:30:14 +10:00
unkin-agent 5928233a97 watchpr: start the conflict window at the arm
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
The window ran from the first non-mergeable poll even while the rule was
disarmed, so the poll that armed it confirmed a run it played no part in.
Measure from the later of the run's start and the arm; re-arming stays a
no-op so a base moving under every poll still confirms.
2026-09-26 22:14:25 +10:00
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 1412 additions and 83 deletions
+46 -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,45 @@ 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. The window starts at the later of the
run's first observation and the arm, which is what keeps arming and the run
independent: movement arms without resetting the run — `base.sha` is the base
branch tip as of the response, so it moves for every open PR whenever main does
and a reset there could never complete on a busy repo — while an arming poll
still cannot confirm a run it played no part in. Re-arming an already-armed
watch is a no-op, so a base moving under every poll advances the window once
and never again. Both times are pointers because the zero `time.Time` is a
legal clock value and cannot also mean "no run". `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 is unreported while the commits stay put, since 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). The commits rarely stay put —
a merge to the base branch moves `base.sha` under every open PR — but that
move only arms; the two minutes it then has to outlast are the recompute the
move started, not the falses before it.
- 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.
+40 -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,38 @@ 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 window runs from whichever came later, so the poll that starts
the merge computation never confirms a run of falses that predates it. 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.
So a conflict introduced by the push immediately before the watch started is not
reported while the commits stay put: nothing in Gitea's payload separates it from
a merge check still in flight, and the baseline line is the only notice of it.
That is a narrower gap than it looks, because `base.sha` is the base branch's
tip — it moves for every open PR whenever anything merges to the base branch, so
the commits rarely stay put for long. The first such move arms the rule, and is
itself silent: the window then runs from that arming poll rather than from the
falses that predate it, so what is eventually reported is a conflict that
outlasted the merge computation the move started.
Arming is a one-way latch. Moves after that one — and every move seen by a watch
that was already armed at the baseline — neither re-arm nor restart the window,
because a restart on every `base.sha` move could never complete on a busy base
branch. That is the cost side of the same trade: on a busy base branch a
conflict can be confirmed while the newest merge recompute is less than two
minutes old. The window guarantees that the run of falses outlasted *a* merge
computation this watch saw start, not that it outlasted the most recent one.
```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)
}
}
}
+102 -13
View File
@@ -1,8 +1,19 @@
// 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. A conflict introduced by the push immediately before the watch started is
// not reported while the commits stay put, because nothing in Gitea's payload
// separates it from a merge check still in flight; the baseline line is the only
// notice of it. The first later move of the head or base arms the rule without
// alerting, and the window then runs from that arm rather than from the falses
// that predate it. Moves after that neither re-arm nor restart the window -- a
// restart on every base move could never complete on a busy base branch -- so a
// conflict can be confirmed while the newest merge recompute is less than two
// minutes old.
//
// watchpr owner/repo#12 owner/repo:15
// watchpr --once --json owner/repo#12
@@ -13,7 +24,9 @@ package main
import (
"encoding/json"
"fmt"
"io"
"os"
"strings"
"time"
"git.unkin.net/unkin/agent-tools/internal/agent"
@@ -43,7 +56,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,
@@ -63,7 +77,7 @@ func newRootCmd() *cobra.Command {
}
refs = append(refs, ref)
}
c := clientFor()
c := clientFor(jsonMode)
if once {
return runOnce(c, refs, jsonMode)
}
@@ -89,15 +103,35 @@ func newRootCmd() *cobra.Command {
// clientFor builds the Gitea client. Watching public repos works anonymously,
// so an unavailable token is a warning, not a failure; a poll that is actually
// rejected re-mints then.
func clientFor() *agent.GiteaClient {
func clientFor(jsonMode bool) *agent.GiteaClient {
token, err := agent.GiteaToken()
if err != nil {
fmt.Fprintf(os.Stderr, "warning: no Gitea token (%v); polling anonymously\n", err)
warn(os.Stderr, jsonMode, "no Gitea token (%v); polling anonymously", err)
token = ""
}
return agent.NewGiteaClient(token)
}
// warnRecord is the --json form of a warning. Under --json every notice watchpr
// itself writes to stderr -- warnings and the baseline -- is an NDJSON record,
// so a caller parsing them line by line never has to guess which shape a line
// is. A terminal failure is the one exception: SilenceErrors stays off, so
// cobra prints it as a plain "Error: ..." line and the exit status is non-zero.
type warnRecord struct {
Warning string `json:"warning"`
}
// warn writes a non-fatal notice to stderr in whichever form the caller asked
// for.
func warn(w io.Writer, jsonMode bool, format string, args ...any) {
msg := fmt.Sprintf(format, args...)
if jsonMode {
_ = json.NewEncoder(w).Encode(warnRecord{Warning: msg})
return
}
_, _ = fmt.Fprintf(w, "warning: %s\n", msg)
}
// runOnce fetches and prints the current state of each PR, then exits 0.
func runOnce(c *agent.GiteaClient, refs []agent.PRRef, jsonMode bool) error {
login := agent.AgentLogin()
@@ -126,13 +160,11 @@ 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)
warn(os.Stderr, jsonMode, "polling %s: %v", ref.String(), err)
}
res, err := agent.Watch(c, refs, login, ticker.C, onBaseline, onError)
@@ -143,6 +175,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 +234,39 @@ 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
switch st.Mergeable {
case agent.MergeNo:
conds = append(conds, "already non-mergeable (a real conflict, or Gitea still recomputing)")
case agent.MergeUnknown:
conds = append(conds, "mergeability unknown (the conflict rule is disarmed until Gitea reports mergeable or the commits move)")
}
if st.CIStatus == "failure" || st.CIStatus == "error" {
conds = append(conds, "CI already "+st.CIStatus)
}
return strings.Join(conds, ", ")
}
func ciOrNone(s string) string {
+129
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,129 @@ 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())
}
}
// Under --json stderr is the baseline and warning channel, so every notice
// watchpr writes there has to be one shape: a caller parsing it line by line
// must never meet a bare `warning:` line between two NDJSON records. Cobra's
// terminal `Error: ...` line is not covered here -- SilenceErrors stays off, so
// it is plain text on stderr alongside a non-zero exit.
func TestJSONModeWarningsAndBaselineAreRecords(t *testing.T) {
st := agent.PRState{
Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7},
State: "open",
Mergeable: agent.MergeNo,
}
var buf bytes.Buffer
warn(&buf, true, "no Gitea token (%v); polling anonymously", errors.New("vault refused"))
emitBaselines(&buf, []agent.PRState{st}, 30*time.Second, true)
warn(&buf, true, "polling %s: %v", st.Ref.String(), errors.New("HTTP 502"))
lines := strings.Split(strings.TrimSpace(buf.String()), "\n")
if len(lines) != 3 {
t.Fatalf("stderr = %q, want 3 records", buf.String())
}
for _, line := range lines {
var rec map[string]any
if err := json.Unmarshal([]byte(line), &rec); err != nil {
t.Errorf("stderr line %q is not a JSON record: %v", line, err)
}
}
var first warnRecord
if err := json.Unmarshal([]byte(lines[0]), &first); err != nil || !strings.Contains(first.Warning, "vault refused") {
t.Errorf("first record = %q, want the token warning", lines[0])
}
buf.Reset()
warn(&buf, false, "polling %s: %v", st.Ref.String(), errors.New("HTTP 502"))
if got := buf.String(); !strings.HasPrefix(got, "warning: ") {
t.Errorf("plain-mode warning = %q, want the warning: prefix", got)
}
}
// An unknown mergeability disarms the conflict rule exactly as a non-mergeable
// baseline does, so the baseline has to name it too.
func TestBaselineNamesAnUnknownMergeability(t *testing.T) {
st := agent.PRState{
Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7},
State: "open",
Mergeable: agent.MergeUnknown,
CIStatus: "success",
}
if got := suppressedAtBaseline(st); !strings.Contains(got, "mergeability unknown") {
t.Errorf("suppressed = %q, want it to name the unknown mergeability", got)
}
if got := baselineLine(st); !strings.Contains(got, "not alerting") {
t.Errorf("baselineLine = %q, want the suppression note", got)
}
}
+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"`
+190 -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,121 @@ 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 and never resets the run, because a base branch that moves
// under the PR on every push to main would otherwise restart it forever. The
// window is instead measured from the later of the run's start and the arm, so
// an arming poll confirms nothing it has not itself outlasted.
type prWatch struct {
prev PRState
armed bool
// armedAt is when the rule armed mid-watch; nil while disarmed, and nil when
// the baseline armed it, since then there is no transition to measure from.
armedAt *time.Time
// conflictSince is when the current unbroken run of non-mergeable polls
// began; nil when no run is in progress.
conflictSince *time.Time
}
func newPRWatch(baseline PRState) *prWatch {
return &prWatch{prev: baseline, armed: baseline.Mergeable == MergeYes}
}
// arm records the disarmed->armed transition and when it happened. Re-arming is
// a no-op, so a base branch moving under every poll advances nothing.
func (w *prWatch) arm(now time.Time) {
if w.armed {
return
}
w.armed = true
at := now
w.armedAt = &at
}
// 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 == nil {
at := now
w.conflictSince = &at
}
case MergeYes:
w.arm(now)
w.conflictSince = nil
default:
w.conflictSince = nil
}
}
// 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 = nil
}
// windowStart is the later of the run's first observation and the arm, so the
// window always covers observations this watch can attribute a merge
// computation to. A baseline arm records no time and leaves the run governing.
func (w *prWatch) windowStart() *time.Time {
if w.conflictSince == nil {
return nil
}
if w.armedAt != nil && w.armedAt.After(*w.conflictSince) {
return w.armedAt
}
return w.conflictSince
}
// confirmed reports whether the run of non-mergeable observations has spanned
// the recompute window since it started counting, so neither a lone
// non-mergeable poll nor the poll that armed the rule confirms anything.
func (w *prWatch) confirmed(now time.Time) bool {
start := w.windowStart()
return start != nil && now.Sub(*start) >= 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.arm(now)
}
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 +274,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 +287,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 +305,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 +315,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 +341,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 +364,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, ""
}
+612 -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,603 @@ 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 base move does not confirm a run it took no part in",
interval: normal, baseline: no, polls: []PRState{no, no, no, movedNo}, want: "",
},
{
name: "a push does not confirm a run it took no part in",
interval: normal, baseline: no, polls: []PRState{no, no, no, pushedNo}, want: "",
},
{
name: "an armed run confirms a window after the arm, not before",
interval: normal, baseline: no,
polls: []PRState{no, no, no, movedNo, movedNo, movedNo}, 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)
}
}
// The window is measured from the arm, not from the run the arming poll walked
// in on: a base branch that moves under a long-conflicted PR arms the rule and
// then has to outlast the recompute itself before anything is reported.
func TestWatchConflictWindowStartsAtTheArm(t *testing.T) {
stuck := base()
stuck.Mergeable = MergeNo
moved := stuck
moved.BaseSHA = "base111"
f := &fakeFetcher{states: []PRState{stuck, stuck, stuck, stuck, moved, moved, moved}}
res, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", spacedTicks(6, time.Minute), 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 two minutes after the base moved", res.Reason)
}
if f.calls != 7 {
t.Errorf("fetch calls = %d, want 7: the window runs from the arming poll, not from the run it inherited", f.calls)
}
}
// The zero time is a legal clock value, so it must not double as the "no run in
// progress" sentinel: a caller whose ticks start at the zero time gets the same
// debounce as any other.
func TestWatchConflictWindowCountsFromTheZeroTime(t *testing.T) {
ok := base()
conflicted := base()
conflicted.Mergeable = MergeNo
ticks := make(chan time.Time, 2)
ticks <- time.Time{}
ticks <- time.Time{}.Add(10 * time.Minute)
close(ticks)
f := &fakeFetcher{states: []PRState{ok, conflicted, conflicted}}
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", ticks, 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 run starting at the zero time still counts", res.Reason)
}
}