21 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
benvin d77607c4f0 Merge pull request 'Name the cause when a watch stops on an auth failure' (#18) from benvin/watchpr-auth-failure into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #18
2026-09-19 17:04:18 +10:00
unkin-agent ff8ac5ea2d Merge main into watchpr auth failure branch
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
Keep both sides' additions to client_test.go and adopt main's
EditPROptions -> EditOptions rename.
2026-09-19 16:40:44 +10:00
benvin 81ae4041cd Merge pull request 'agentws: check out --branch from origin when it exists' (#17) from benvin/agentws-branch-checkout into main
Reviewed-on: #17
2026-09-19 16:36:45 +10:00
benvin 2610ea5c09 Merge pull request 'Add agentpr issue create/comment/edit' (#16) from benvin/agentpr-issue into main
Reviewed-on: #16
2026-09-19 16:28:33 +10:00
unkin-agent 1b90e60aeb Run go test once per module in pre-commit
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
The go-unit-tests hook caps every package at 30s and re-runs the whole module
once per file batch, so the git-fixture tests in cmd/agentws timed out as soon
as they grew.

- Replace go-unit-tests with a module-level `go test ./...` local hook
2026-09-19 16:20:36 +10:00
unkin-agent 90ce747a61 Name the cause when a watch stops on an auth failure
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
A 401/403 was handled as one thing, so watchpr re-minted on every
rejection and reported "token expired" for a permission boundary or an
anonymous run that had no token to expire, sending the reader after the
wrong problem.

Classify a 401/403 as a rejected credential, a permission denial, or a
request that carried no token, and re-mint only the first.
Reject a re-minted empty token instead of replaying anonymously.
Report the classified cause from --once as well as from the watch loop.
Document watchpr's exit behaviour per cause.
2026-09-19 16:13:38 +10:00
unkin-agent 7510187243 agentws: check out --branch from origin when it exists
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline failed
`agentws new --branch` always forked from the base branch, so a worktree for a
branch that already existed on origin started at the base's HEAD with none of
the branch's commits, and callers had to reset --hard afterwards to recover.

- Check out an existing origin/<branch> and set the branch to track it
- Fast-forward a stale local branch onto origin, keeping unpushed commits
- Fork from --from or the remote default only when origin lacks the branch
- Name which of the two paths was taken in the command's output
- Ignore --from, with a note, when the branch is already on origin
2026-09-19 16:11:59 +10:00
unkin-agent 3de9d35699 Add agentpr issue create/comment/edit
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Filing an issue as unkin-agent means hand-rolling a POST to the issues
API with a manually minted token, the exact credential handling agentpr
exists to encapsulate.

- add `issue create`, `issue comment` and `issue edit`, mirroring the pr
  command group's flags, output and errors
- add CreateIssue/EditIssue to the Gitea client, sharing EditOptions with
  EditPR
- build both comment commands from one constructor so pr and issue
  comments cannot drift off Gitea's shared endpoint
- cover create, edit, a missing repo, an API error and missing flags
2026-09-19 16:11:42 +10:00
benvin 72adebbf8b Merge pull request 'Add agentpr pr edit subcommand' (#15) from benvin/agentpr-pr-edit into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #15
2026-09-19 13:48:44 +10:00
unkin-agent 3f990c841d Mock the 404 body Gitea really returns
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
2026-09-19 12:44:00 +10:00
unkin-agent 78d83b7a61 Reject an empty pr edit title Gitea would ignore
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline failed
2026-09-19 12:38:12 +10:00
unkin-agent 7bc4082cb0 Add agentpr pr edit subcommand
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
Update a PR's title and/or body as the agent user, sending only the
fields supplied.
2026-09-19 12:32:07 +10:00
14 changed files with 2508 additions and 122 deletions
+10 -3
View File
@@ -13,10 +13,11 @@ repos:
rev: v0.5.1
hooks:
- id: go-fmt
- id: go-unit-tests
# go-vet at the module level (dnephin's go-vet runs at repo root, which has no
# .go files here since both tools live under cmd/). The CI pre-commit image
# go vet and go test at the module level (dnephin's run at repo root, which has
# no .go files here since both tools live under cmd/, and its go-unit-tests
# caps every package at 30s and re-runs the whole module once per file batch —
# the git-fixture tests outgrew both). The CI pre-commit image
# (almalinux9-gobuilder) has go installed.
- repo: local
hooks:
@@ -26,3 +27,9 @@ repos:
language: system
types: [go]
pass_filenames: false
- id: go-test-mod
name: go test (module)
entry: go test ./...
language: system
types: [go]
pass_filenames: false
+57 -11
View File
@@ -8,9 +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 pull requests and post PR comments as `unkin-agent`
(fixes the "tea posts as Ben" attribution problem). Subcommands:
`pr create`, `pr comment`, `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
@@ -28,7 +30,7 @@ parsing, watch-state comparison, git worktree helpers).
## Structure
```
cmd/agentpr/main.go # agentpr CLI (pr create / pr comment / 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)
@@ -36,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 create/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)
@@ -129,8 +131,9 @@ 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 create /
comment / whoami / status). No live Vault/Gitea access is required for tests.
against `httptest` servers (fake AppRole login + gitea creds + PR/issue create
+ edit / close / reopen / comment / whoami / status). No live Vault/Gitea access
is required for tests.
## agentvault seed-outpost
@@ -175,11 +178,22 @@ wrapped per stage (login / read denied / write denied) via `ErrVaultDenied`.
## Gotchas
- `watchpr` exits 0 with no output changes on `--once` (just prints state).
- Gitea tokens expire in ~1h, shorter than a watch: the client re-mints once on a
401/403 and replays the request. If the fresh token is rejected too, `watchpr`
exits non-zero rather than polling blind.
- Gitea tokens expire in ~1h, shorter than a watch: the client re-mints once when
the credential it sent was rejected and replays the request. If the fresh token
is rejected too, `watchpr` exits non-zero rather than polling blind.
- A 401/403 is classified before anything is re-minted, because only one of the
three cases is a stale token: `ErrNoCredential` (the request carried no token —
anonymous access to something not public), `IsPermissionDenied` (a 403 whose
body names no credential, so the identity is simply not allowed) and
`IsCredentialRejected` (any 401, or a 403 mentioning a token/scope/sign-in,
which is what Gitea returns for a token missing a scope). Only the last
re-mints; the others abort immediately, since a fresh token cannot fix them and
blaming one hides the real cause. `IsAuthError` stays "any 401/403" — all three
end a watch.
- `watchpr` polls anonymously when no token can be minted (public repos work
fine); only a real 401/403 reaches for Vault.
fine); an anonymous run never reaches for Vault, on any status code.
- A re-mint that hands back an empty token is an error: replaying with it would
drop the Authorization header and silently continue as an anonymous watcher.
- The token cache is process-wide (mutex-guarded); `RefreshGiteaToken` replaces
it. Tests call the unexported `fetchGiteaToken` to avoid the cache.
- `agentvault` never puts a secret in an error string: Vault decode failures and
@@ -206,5 +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.
+78 -4
View File
@@ -6,7 +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 pull requests and post PR comments 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
@@ -50,6 +51,29 @@ agentpr pr create --repo unkin/argocd-apps \
# Comment on a PR
agentpr pr comment --repo unkin/argocd-apps --pr 42 --body "Rebased, CI green."
# Edit a PR's title and/or body; an omitted flag is left unchanged
agentpr pr edit --repo unkin/argocd-apps --pr 42 --body "Adds the ServiceAccount ..."
agentpr pr edit --repo unkin/argocd-apps --pr 42 --title "Add woodpecker SA"
# prints: #<number> <html_url>
# File an issue (--body optional)
agentpr issue create --repo unkin/argocd-apps \
--title "Woodpecker SA missing" --body "The pipeline fails with ..."
# prints: #<number> <html_url>
# Comment on an issue (the same Gitea endpoint `pr comment` posts to)
agentpr issue comment --repo unkin/argocd-apps --issue 43 --body "Fixed in #44."
# Edit an issue's title and/or body; an omitted flag is left unchanged
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
```
@@ -61,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)
@@ -81,6 +122,25 @@ watchpr --once --json unkin/argocd-apps#42
On a meaningful change `watchpr` prints the reason and the PR's current state,
then exits 0. Use `--json` for machine-readable output.
### Exit behaviour
A watcher that sees nothing must not look healthy, so every terminal failure
exits non-zero naming its cause:
| Cause | Message |
|---|---|
| the token was rejected and a fresh one was too | `gitea rejected the token and re-minting did not recover it` |
| authenticated but not allowed (Gitea 403, no token named) | `gitea denied access to <login>` |
| polling anonymously and the PR is not public | `gitea requires authentication and no token could be minted` |
| the PR lookup 404s (repo deleted, renamed, made private) | `PR no longer visible` |
Gitea tokens expire in ~1h, far shorter than a watch, so a rejected token is
re-minted once and the request replayed; only a failure that survives that
re-mint ends the watch. Anonymous polling of a public repo is unaffected — with
no token there is nothing to reject and Vault is never reached for one.
Transient failures (5xx, network errors, rate limiting) are warned about and
retried, and give up after 20 consecutive failures of the same PR.
## agentws
`agentws` gives an agent an isolated git worktree per branch without disturbing
@@ -91,10 +151,10 @@ checkout too; the worktrees themselves live under the **worktree root**
```bash
# Clone unkin/argocd-apps into ~/src/prodenv if missing, then add a worktree for
# a new branch off the remote default branch. Prints the worktree path.
# the branch. Prints the worktree path.
agentws new argocd-apps --branch benvin/my-change
# Branch off a specific base instead of the remote default
# Branch off a specific base instead of the remote default (new branches only)
agentws new argocd-apps --branch benvin/hotfix --from release-1.2
# List managed worktrees (repo, branch, path)
@@ -118,6 +178,20 @@ agentws clean
agentws token
```
`agentws new` fetches first, then takes one of two paths and names the one it
took on its last output line. A branch that **already exists on origin** is
checked out at `origin/<branch>` and set to track it, so the worktree starts on
the branch's own commits (`branch <b> (tracking origin/<b> at <sha>)`); a local
branch left from an earlier run is fast-forwarded onto it. A branch origin does
**not** have is created from `--from`, or from the remote's default branch when
`--from` is absent (`branch <b> (new, from origin/<base>)`) — the default is read
from `origin/HEAD`, so a repo on `master` forks from `master`. `--from` is
ignored, with a note, when the branch is already on origin.
The one case the worktree does not land on `origin/<branch>` is a local branch
carrying commits origin has never seen. Those commits exist nowhere else, so the
checkout is left on them and the output says how many.
### prune
`agentws prune` finds worktrees two ways and merges the results: the managed
+218 -14
View File
@@ -5,6 +5,12 @@
//
// agentpr pr create --repo owner/repo --base main --head feature --title T --body B
// agentpr pr comment --repo owner/repo --pr 12 --body "..."
// agentpr pr edit --repo owner/repo --pr 12 --title T --body B
// 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
@@ -33,14 +39,14 @@ func main() {
func newRootCmd() *cobra.Command {
root := &cobra.Command{
Use: "agentpr",
Short: "Manage Gitea PRs and comments as an agent user.",
Long: "agentpr manages Gitea pull requests and comments as an agent user, using a\nGitea token minted from Vault (AppRole login + gitea/creds/<AGENT_LOGIN>).\nSet AGENT_LOGIN to act as another agent identity, or GITEA_CREDS_PATH to name\nthe Vault creds path outright.",
Short: "Manage Gitea PRs, issues and comments as an agent user.",
Long: "agentpr manages Gitea pull requests, issues and comments as an agent user,\nusing a Gitea token minted from Vault (AppRole login + gitea/creds/<AGENT_LOGIN>).\nSet AGENT_LOGIN to act as another agent identity, or GITEA_CREDS_PATH to name\nthe Vault creds path outright.",
Version: version,
SilenceUsage: true,
}
root.SetVersionTemplate("{{.Version}}\n")
root.AddCommand(newPRCmd(), newWhoamiCmd(), newVersionCmd())
root.AddCommand(newPRCmd(), newIssueCmd(), newWhoamiCmd(), newVersionCmd())
return root
}
@@ -56,9 +62,24 @@ func client() (*agent.GiteaClient, error) {
func newPRCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "pr",
Short: "Create PRs and post PR comments",
Short: "Create and edit PRs, and post PR comments",
}
cmd.AddCommand(newPRCreateCmd(), newPRCommentCmd())
cmd.AddCommand(newPRCreateCmd(), newCommentCmd("pr", "PR", "Post a comment on a pull request"), newPREditCmd())
return cmd
}
func newIssueCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "issue",
Short: "File, edit, close and reopen issues, and post issue comments",
}
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
}
@@ -103,12 +124,56 @@ func newPRCreateCmd() *cobra.Command {
return cmd
}
func newPRCommentCmd() *cobra.Command {
// newCommentCmd builds a comment command whose number flag is named numFlag.
// Gitea backs every PR with an issue of the same number and serves comments
// from the issue endpoint, so `pr comment` and `issue comment` are one command
// under two flag names rather than two implementations that could drift.
func newCommentCmd(numFlag, noun, short string) *cobra.Command {
var repo, body string
var pr int
var number int
cmd := &cobra.Command{
Use: "comment",
Short: "Post a comment on a pull request",
Short: short,
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if number <= 0 {
return fmt.Errorf("--%s must be a positive %s number", numFlag, noun)
}
if body == "" {
return fmt.Errorf("--body is required")
}
c, err := client()
if err != nil {
return err
}
cm, err := c.CreateComment(owner+"/"+name, number, body)
if err != nil {
return err
}
fmt.Printf("comment %d posted on %s/%s#%d\n", cm.ID, owner, name, number)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&number, numFlag, 0, noun+" number (required)")
f.StringVar(&body, "body", "", "Comment body (required)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired(numFlag)
_ = cmd.MarkFlagRequired("body")
return cmd
}
func newPREditCmd() *cobra.Command {
var repo, title, body string
var pr int
cmd := &cobra.Command{
Use: "edit",
Short: "Edit a pull request's title and/or body",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
@@ -118,28 +183,167 @@ func newPRCommentCmd() *cobra.Command {
if pr <= 0 {
return fmt.Errorf("--pr must be a positive PR number")
}
if body == "" {
return fmt.Errorf("--body is required")
opts, err := editOptions(cmd, title, body)
if err != nil {
return err
}
c, err := client()
if err != nil {
return err
}
cm, err := c.CreateComment(owner+"/"+name, pr, body)
updated, err := c.EditPR(owner+"/"+name, pr, opts)
if err != nil {
return err
}
fmt.Printf("comment %d posted on %s/%s#%d\n", cm.ID, owner, name, pr)
fmt.Printf("#%d %s\n", updated.Number, updated.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&pr, "pr", 0, "PR number (required)")
f.StringVar(&body, "body", "", "Comment body (required)")
f.StringVar(&title, "title", "", "New PR title (unchanged when omitted)")
f.StringVar(&body, "body", "", "New PR body (unchanged when omitted)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("pr")
_ = cmd.MarkFlagRequired("body")
return cmd
}
// editOptions turns the --title/--body flags actually given into an edit
// payload. Only the flags present are sent: omitting --title must leave the
// title as it is, not blank it.
func editOptions(cmd *cobra.Command, title, body string) (agent.EditOptions, error) {
var opts agent.EditOptions
if cmd.Flags().Changed("title") {
// Gitea ignores an empty title, so sending one would report success
// while changing nothing.
if title == "" {
return opts, fmt.Errorf("--title cannot be empty: a title can be set but not cleared")
}
opts.Title = &title
}
if cmd.Flags().Changed("body") {
opts.Body = &body
}
if opts.Title == nil && opts.Body == nil {
return opts, fmt.Errorf("at least one of --title or --body is required")
}
return opts, nil
}
func newIssueCreateCmd() *cobra.Command {
var repo, title, body string
cmd := &cobra.Command{
Use: "create",
Short: "File an issue",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if title == "" {
return fmt.Errorf("--title is required")
}
c, err := client()
if err != nil {
return err
}
issue, err := c.CreateIssue(owner+"/"+name, agent.CreateIssueOptions{
Title: title,
Body: body,
})
if err != nil {
return err
}
fmt.Printf("#%d %s\n", issue.Number, issue.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.StringVar(&title, "title", "", "Issue title (required)")
f.StringVar(&body, "body", "", "Issue body")
_ = cmd.MarkFlagRequired("repo")
return cmd
}
func newIssueEditCmd() *cobra.Command {
var repo, title, body string
var issue int
cmd := &cobra.Command{
Use: "edit",
Short: "Edit an issue's title and/or body",
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")
}
opts, err := editOptions(cmd, title, body)
if err != nil {
return err
}
c, err := client()
if err != nil {
return err
}
updated, err := c.EditIssue(owner+"/"+name, issue, opts)
if err != nil {
return err
}
fmt.Printf("#%d %s\n", updated.Number, updated.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&issue, "issue", 0, "Issue number (required)")
f.StringVar(&title, "title", "", "New issue title (unchanged when omitted)")
f.StringVar(&body, "body", "", "New issue body (unchanged when omitted)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("issue")
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
}
+186
View File
@@ -2,7 +2,10 @@ package main
import (
"io"
"strings"
"testing"
"github.com/spf13/cobra"
)
// A malformed --repo must fail the command (so main exits non-zero). ParseRepo
@@ -16,3 +19,186 @@ func TestExecuteBadRepoErrors(t *testing.T) {
t.Fatal("Execute() = nil, want error for a malformed --repo")
}
}
// `pr edit` with neither --title nor --body has nothing to send; it must fail
// with a usage error before any Vault/Gitea call, so this stays hermetic.
func TestPREditRequiresTitleOrBody(t *testing.T) {
cmd := newRootCmd()
cmd.SetArgs([]string{"pr", "edit", "--repo", "unkin/repo", "--pr", "7"})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if err == nil {
t.Fatal("Execute() = nil, want an error when neither --title nor --body is given")
}
if !strings.Contains(err.Error(), "--title or --body") {
t.Errorf("error = %q, want it to name the missing flags", err)
}
}
// Gitea silently ignores an empty title, so `pr edit --title ""` would report
// success while changing nothing; it must fail before any Vault/Gitea call.
func TestPREditRejectsEmptyTitle(t *testing.T) {
cmd := newRootCmd()
cmd.SetArgs([]string{"pr", "edit", "--repo", "unkin/repo", "--pr", "7", "--title", ""})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if err == nil {
t.Fatal("Execute() = nil, want an error for an empty --title")
}
if !strings.Contains(err.Error(), "--title cannot be empty") {
t.Errorf("error = %q, want it to reject the empty title", err)
}
}
// execute runs the command tree with args, discarding output, so tests assert
// on the error alone. Every case here fails before any Vault/Gitea call.
func execute(args ...string) error {
cmd := newRootCmd()
cmd.SetArgs(args)
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
return cmd.Execute()
}
// An issue needs a title; Gitea rejects an empty one, so the command must too.
func TestIssueCreateRequiresTitle(t *testing.T) {
err := execute("issue", "create", "--repo", "unkin/repo", "--body", "b")
if err == nil {
t.Fatal("Execute() = nil, want an error when --title is missing")
}
if !strings.Contains(err.Error(), "--title is required") {
t.Errorf("error = %q, want it to name the missing flag", err)
}
}
// --repo is required, and cobra must reject its absence before anything reaches
// for a token.
func TestIssueCreateRequiresRepo(t *testing.T) {
err := execute("issue", "create", "--title", "t")
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 flag", err)
}
}
func TestIssueCreateBadRepoErrors(t *testing.T) {
if err := execute("issue", "create", "--repo", "not-a-repo", "--title", "t"); err == nil {
t.Fatal("Execute() = nil, want error for a malformed --repo")
}
}
// `issue comment` addresses the issue by --issue, not --pr, and needs it.
func TestIssueCommentRequiresIssueNumber(t *testing.T) {
err := execute("issue", "comment", "--repo", "unkin/repo", "--body", "hi")
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)
}
}
func TestIssueEditRequiresTitleOrBody(t *testing.T) {
err := execute("issue", "edit", "--repo", "unkin/repo", "--issue", "12")
if err == nil {
t.Fatal("Execute() = nil, want an error when neither --title nor --body is given")
}
if !strings.Contains(err.Error(), "--title or --body") {
t.Errorf("error = %q, want it to name the missing flags", err)
}
}
func TestIssueEditRejectsEmptyTitle(t *testing.T) {
err := execute("issue", "edit", "--repo", "unkin/repo", "--issue", "12", "--title", "")
if err == nil {
t.Fatal("Execute() = nil, want an error for an empty --title")
}
if !strings.Contains(err.Error(), "--title cannot be empty") {
t.Errorf("error = %q, want it to reject the empty title", err)
}
}
// PRs and issues share Gitea's comment endpoint, so both comment commands are
// built from one constructor: they must stay identical apart from the flag
// naming the number.
func TestCommentCommandsStayInStep(t *testing.T) {
find := func(group string) *cobra.Command {
t.Helper()
cmd, _, err := newRootCmd().Find([]string{group, "comment"})
if err != nil || cmd.Name() != "comment" {
t.Fatalf("%s comment not found: %v", group, err)
}
return cmd
}
has := func(cmd *cobra.Command, name string) bool { return cmd.Flags().Lookup(name) != nil }
prCmd, issueCmd := find("pr"), find("issue")
for _, name := range []string{"repo", "body"} {
if !has(prCmd, name) || !has(issueCmd, name) {
t.Errorf("--%s missing: pr=%t issue=%t", name, has(prCmd, name), has(issueCmd, name))
}
}
if !has(prCmd, "pr") || has(prCmd, "issue") {
t.Error("pr comment must take --pr and only --pr")
}
if !has(issueCmd, "issue") || has(issueCmd, "pr") {
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)
}
}
}
+70 -8
View File
@@ -161,13 +161,24 @@ func newNewCmd() *cobra.Command {
return err
}
// c. Base branch: --from or the remote default.
// c. A branch origin already has is work in progress and must be
// checked out as it stands; only a branch origin does not have is
// forked from a base.
onRemote := agent.GitRemoteBranchExists(srcDir, "origin", branch)
startPoint := "origin/" + branch
base := from
if base == "" {
base, err = agent.GitRemoteDefaultBranch(srcDir, "origin")
if err != nil {
return err
if onRemote {
if from != "" {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "note: --from %s ignored, origin/%s already exists\n", from, branch)
}
} else {
if base == "" {
base, err = agent.GitRemoteDefaultBranch(srcDir, "origin")
if err != nil {
return err
}
}
startPoint = "origin/" + base
}
// d. Create the worktree FROM the source checkout so the branch is
@@ -176,7 +187,7 @@ func newNewCmd() *cobra.Command {
if _, statErr := os.Stat(wtPath); statErr == nil {
return fmt.Errorf("worktree already exists at %s", wtPath)
}
if err := agent.GitWorktreeAdd(srcDir, wtPath, branch, "origin/"+base); err != nil {
if err := agent.GitWorktreeAdd(srcDir, wtPath, branch, startPoint); err != nil {
return err
}
@@ -199,9 +210,19 @@ func newNewCmd() *cobra.Command {
return err
}
// f. Report the worktree path and branch.
// f. A local branch left from an earlier run may sit behind origin,
// so reusing it is not enough on its own.
summary := fmt.Sprintf("branch %s (new, from origin/%s)", branch, base)
if onRemote {
summary, err = alignToRemote(cmd.OutOrStdout(), wtPath, branch)
if err != nil {
return err
}
}
// g. Report the worktree path and which of the two paths was taken.
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", wtPath)
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "branch %s (from origin/%s)\n", branch, base)
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", summary)
return nil
},
}
@@ -212,6 +233,47 @@ func newNewCmd() *cobra.Command {
return cmd
}
// alignToRemote puts the worktree on origin/<branch> and reports what that took.
// A reused local branch can be stale, and a fast-forward is the only move that
// adds no commit and drops none; a local branch carrying commits origin does not
// have is left where it stands, because those commits exist nowhere else.
func alignToRemote(out io.Writer, wtPath, branch string) (string, error) {
remoteRef := "origin/" + branch
want, err := agent.GitRevParse(wtPath, remoteRef)
if err != nil {
return "", err
}
head, err := agent.GitRevParse(wtPath, "HEAD")
if err != nil {
return "", err
}
if head != want {
ahead, err := agent.GitAheadCount(wtPath, remoteRef, "HEAD")
if err != nil {
return "", err
}
if ahead > 0 {
return fmt.Sprintf("branch %s (local, %s not on %s, left at %s)",
branch, commitCount(ahead), remoteRef, shortSHA(head)), nil
}
if err := agent.GitMergeFFOnly(wtPath, remoteRef); err != nil {
return "", err
}
_, _ = fmt.Fprintf(out, "fast-forwarded stale %s to %s\n", branch, remoteRef)
}
if err := agent.GitSetUpstream(wtPath, branch, remoteRef); err != nil {
return "", err
}
return fmt.Sprintf("branch %s (tracking %s at %s)", branch, remoteRef, shortSHA(want)), nil
}
func shortSHA(sha string) string {
if len(sha) > 7 {
return sha[:7]
}
return sha
}
// --- list -----------------------------------------------------------------
func newListCmd() *cobra.Command {
+192
View File
@@ -0,0 +1,192 @@
package main
import (
"bytes"
"path/filepath"
"strings"
"testing"
"git.unkin.net/unkin/agent-tools/internal/agent"
)
// newFixtureOn builds the same origin/source/worktree-root layout as the prune
// fixture but with a chosen default branch, so `new` can be tested against a
// repo whose default is not "main".
func newFixtureOn(t *testing.T, defBranch string) *fixture {
t.Helper()
root := t.TempDir()
f := &fixture{
root: root,
bare: filepath.Join(root, "origin.git"),
srcDir: filepath.Join(root, "src", "repo"),
wtRoot: filepath.Join(root, "worktrees"),
}
git(t, root, "init", "--bare", "-b", defBranch, f.bare)
seed := filepath.Join(root, "seed")
git(t, root, "init", "-b", defBranch, seed)
identity(t, seed)
writeCommit(t, seed, "README.md", "hi\n", "init")
git(t, seed, "remote", "add", "origin", f.bare)
git(t, seed, "push", "-u", "origin", defBranch)
git(t, root, "clone", f.bare, f.srcDir)
identity(t, f.srcDir)
t.Setenv("AGENTWS_ROOT", f.wtRoot)
t.Setenv("AGENTWS_SRC_ROOT", filepath.Join(root, "src"))
t.Setenv("AGENTWS_OWNER", "unkin")
return f
}
// runNew invokes `agentws new repo --branch <branch>` and returns its output.
func runNew(t *testing.T, branch string, extra ...string) string {
t.Helper()
var out bytes.Buffer
cmd := newRootCmd()
cmd.SetArgs(append([]string{"new", "repo", "--branch", branch}, extra...))
cmd.SetOut(&out)
cmd.SetErr(&out)
if err := cmd.Execute(); err != nil {
t.Fatalf("new %s: %v (output %q)", branch, err, out.String())
}
return out.String()
}
// pushBranch creates branch on origin carrying one commit and returns its SHA.
func pushBranch(t *testing.T, f *fixture, branch, file, content string) string {
t.Helper()
seed := filepath.Join(f.root, "seed")
git(t, seed, "checkout", "-b", branch)
writeCommit(t, seed, file, content, "work on "+branch)
git(t, seed, "push", "origin", branch)
return git(t, seed, "rev-parse", "HEAD")
}
func wtPathFor(f *fixture, branch string) string {
return filepath.Join(f.wtRoot, agent.WorktreeDirName("repo", branch))
}
// The regression: a branch that already exists on origin must be checked out at
// origin's tip, not forked from the default branch.
func TestNewChecksOutExistingRemoteBranch(t *testing.T) {
f := newFixtureOn(t, "main")
want := pushBranch(t, f, "benvin/existing", "a.txt", "a\n")
out := runNew(t, "benvin/existing")
path := wtPathFor(f, "benvin/existing")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/existing %s", got, want)
}
if upstream := git(t, path, "rev-parse", "--abbrev-ref", "HEAD@{upstream}"); upstream != "origin/benvin/existing" {
t.Errorf("upstream = %q, want origin/benvin/existing", upstream)
}
if !contains(out, "tracking origin/benvin/existing") {
t.Errorf("output %q does not say the remote branch was checked out", out)
}
}
// A branch origin does not have is still forked from the default branch, and the
// output must say so rather than leaving the caller to guess.
func TestNewForksBranchMissingFromRemote(t *testing.T) {
f := newFixtureOn(t, "main")
want := git(t, f.srcDir, "rev-parse", "origin/main")
out := runNew(t, "benvin/fresh")
path := wtPathFor(f, "benvin/fresh")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/main %s", got, want)
}
if !contains(out, "branch benvin/fresh (new, from origin/main)") {
t.Errorf("output %q does not report a new branch", out)
}
}
// The base is the remote's own default branch, so a repo defaulting to master
// forks from master.
func TestNewForksFromMasterDefaultBranch(t *testing.T) {
f := newFixtureOn(t, "master")
want := git(t, f.srcDir, "rev-parse", "origin/master")
out := runNew(t, "benvin/on-master")
path := wtPathFor(f, "benvin/on-master")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/master %s", got, want)
}
if !contains(out, "from origin/master") {
t.Errorf("output %q does not name origin/master as the base", out)
}
}
// An existing remote branch beats --from: the flag is ignored and the caller is
// told, rather than the branch being silently re-forked.
func TestNewIgnoresFromWhenBranchIsOnRemote(t *testing.T) {
f := newFixtureOn(t, "main")
want := pushBranch(t, f, "benvin/with-from", "a.txt", "a\n")
out := runNew(t, "benvin/with-from", "--from", "main")
path := wtPathFor(f, "benvin/with-from")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/with-from %s", got, want)
}
if !contains(out, "--from main ignored") {
t.Errorf("output %q does not report the ignored --from", out)
}
}
// A local branch left behind by an earlier run must not pin the worktree to a
// commit origin has moved past.
func TestNewFastForwardsStaleLocalBranch(t *testing.T) {
f := newFixtureOn(t, "main")
stale := pushBranch(t, f, "benvin/stale", "a.txt", "a\n")
git(t, f.srcDir, "fetch", "origin")
git(t, f.srcDir, "branch", "benvin/stale", "origin/benvin/stale")
seed := filepath.Join(f.root, "seed")
writeCommit(t, seed, "b.txt", "b\n", "more work")
git(t, seed, "push", "origin", "benvin/stale")
want := git(t, seed, "rev-parse", "HEAD")
if want == stale {
t.Fatal("fixture did not move origin/benvin/stale on")
}
out := runNew(t, "benvin/stale")
path := wtPathFor(f, "benvin/stale")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/stale %s", got, want)
}
if !contains(out, "fast-forwarded stale benvin/stale") {
t.Errorf("output %q does not report the fast-forward", out)
}
}
// A local branch carrying commits origin does not have keeps them: they exist
// nowhere else, so the worktree stays put and the output says so.
func TestNewKeepsLocalCommitsAheadOfRemote(t *testing.T) {
f := newFixtureOn(t, "main")
pushBranch(t, f, "benvin/ahead", "a.txt", "a\n")
git(t, f.srcDir, "fetch", "origin")
git(t, f.srcDir, "checkout", "-b", "benvin/ahead", "origin/benvin/ahead")
writeCommit(t, f.srcDir, "local.txt", "local\n", "local only")
want := git(t, f.srcDir, "rev-parse", "HEAD")
git(t, f.srcDir, "checkout", "main")
out := runNew(t, "benvin/ahead")
path := wtPathFor(f, "benvin/ahead")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want the local tip %s", got, want)
}
if !contains(out, "1 commit not on origin/benvin/ahead") {
t.Errorf("output %q does not report the unpushed commit", out)
}
}
func contains(haystack, needle string) bool {
return strings.Contains(haystack, needle)
}
+91 -17
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,
@@ -105,7 +115,7 @@ func runOnce(c *agent.GiteaClient, refs []agent.PRRef, jsonMode bool) error {
for _, ref := range refs {
st, err := agent.FetchState(c, ref, login)
if err != nil {
return err
return describeFailure(err)
}
states = append(states, st)
}
@@ -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)
@@ -137,18 +145,56 @@ func runWatch(c *agent.GiteaClient, refs []agent.PRRef, interval time.Duration,
res, err := agent.Watch(c, refs, login, ticker.C, onBaseline, onError)
if err != nil {
if agent.IsAuthError(err) {
return fmt.Errorf("gitea authentication failed after re-minting the token, watch aborted: %w", err)
}
if agent.IsPRGone(err) {
return fmt.Errorf("PR no longer visible (repo deleted, renamed, or made private), watch aborted: %w", err)
}
return err
return describeFailure(err)
}
report(res.Ref.String(), res.Reason, res.State, jsonMode)
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
// fixed by a fresh token.
func describeFailure(err error) error {
switch {
case agent.IsNoCredential(err):
return fmt.Errorf("gitea requires authentication and no token could be minted, aborted: %w", err)
case agent.IsPermissionDenied(err):
return fmt.Errorf("gitea denied access to %s (a fresh token will not help), aborted: %w", agent.AgentLogin(), err)
case agent.IsAuthError(err):
return fmt.Errorf("gitea rejected the token and re-minting did not recover it, aborted: %w", err)
case agent.IsPRGone(err):
return fmt.Errorf("PR no longer visible (repo deleted, renamed, or made private), aborted: %w", err)
}
return err
}
// report emits the change that ended the watch.
func report(key, reason string, st agent.PRState, jsonMode bool) {
if jsonMode {
@@ -164,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 {
+151
View File
@@ -1,11 +1,18 @@
package main
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
"git.unkin.net/unkin/agent-tools/internal/agent"
)
// A bad PR reference must fail the command (so main exits non-zero) rather than
@@ -105,3 +112,147 @@ func TestExecuteBadIntervalErrors(t *testing.T) {
}
}
}
// failingVault serves an AppRole login that never issues a token, so the
// command falls back to anonymous polling exactly as it does when Vault is
// unreachable.
func failingVault(t *testing.T) *httptest.Server {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
}))
t.Cleanup(srv.Close)
return srv
}
// An anonymous run against a repo that is not public must exit non-zero saying
// no token was available — not claim a token expired, and not keep going.
func TestOnceAnonymousRejectionNamesTheMissingToken(t *testing.T) {
vault := failingVault(t)
requests := 0
gitea := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requests++
w.WriteHeader(http.StatusUnauthorized)
_, _ = io.WriteString(w, `{"message":"invalid username, password or token"}`)
}))
defer gitea.Close()
t.Setenv("VAULT_ADDR", vault.URL)
t.Setenv("GITEA_URL", gitea.URL)
cmd := newRootCmd()
cmd.SetArgs([]string{"--once", "unkin/repo#7"})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if err == nil {
t.Fatal("Execute() = nil, want a non-zero exit when the poll is rejected")
}
if !strings.Contains(err.Error(), "no token could be minted") {
t.Errorf("Execute() error = %q, want it to name the missing token", err)
}
if requests != 1 {
t.Errorf("gitea requests = %d, want 1 (no replay without a credential)", requests)
}
}
// describeFailure must tell the four terminal causes apart: each one sends the
// reader somewhere different, and a watcher that stops without saying why is
// the failure this names.
func TestDescribeFailureNamesTheCause(t *testing.T) {
rejected := &agent.APIError{Method: "GET", Path: "/p", StatusCode: 401, Body: `{"message":"invalid username, password or token"}`}
forbidden := &agent.APIError{Method: "GET", Path: "/p", StatusCode: 403, Body: `{"message":"Forbidden"}`}
tests := []struct {
name string
err error
want string
}{
{"anonymous", fmt.Errorf("%w: %w", agent.ErrNoCredential, rejected), "no token could be minted"},
{"permission boundary", error(forbidden), "denied access"},
{"rejected token", error(rejected), "re-minting did not recover it"},
{"other", errors.New("dial tcp: timeout"), "dial tcp: timeout"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := describeFailure(tt.err)
if got == nil || !strings.Contains(got.Error(), tt.want) {
t.Errorf("describeFailure = %v, want it to mention %q", got, tt.want)
}
if !errors.Is(got, tt.err) {
t.Errorf("describeFailure dropped the underlying error %v", tt.err)
}
})
}
}
// 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())
}
}
+543
View File
@@ -9,6 +9,7 @@ import (
"net/http/httptest"
"strings"
"testing"
"time"
)
// fakeVault serves the AppRole login and the gitea creds secret at credsPath
@@ -98,6 +99,80 @@ func TestCreatePRRequestBody(t *testing.T) {
}
}
// An edit must send only the fields it was given: Gitea overwrites whatever
// key it receives, so an omitted --title arriving as "" would blank the title.
func TestEditPRSendsOnlySuppliedFields(t *testing.T) {
title, body, empty := "new title", "new body", ""
tests := []struct {
name string
opts EditOptions
want map[string]any
}{
{"body only", EditOptions{Body: &body}, map[string]any{"body": "new body"}},
{"title only", EditOptions{Title: &title}, map[string]any{"title": "new title"}},
{"both", EditOptions{Title: &title, Body: &body}, map[string]any{"title": "new title", "body": "new body"}},
{"explicit empty body is sent", EditOptions{Body: &empty}, map[string]any{"body": ""}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var gotBody map[string]any
var gotMethod, gotPath string
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
gotMethod, gotPath = r.Method, r.URL.Path
_ = json.NewDecoder(r.Body).Decode(&gotBody)
_, _ = io.WriteString(w, `{"number":7,"title":"new title","html_url":"https://git.unkin.net/unkin/repo/pulls/7"}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "gitea-abc", HTTP: srv.Client()}
pr, err := c.EditPR("unkin/repo", 7, tt.opts)
if err != nil {
t.Fatalf("EditPR: %v", err)
}
if gotMethod != http.MethodPatch {
t.Errorf("method = %s, want PATCH", gotMethod)
}
if gotPath != "/api/v1/repos/unkin/repo/pulls/7" {
t.Errorf("path = %q", gotPath)
}
if len(gotBody) != len(tt.want) {
t.Errorf("payload = %v, want exactly the supplied fields %v", gotBody, tt.want)
}
for k, v := range tt.want {
if gotBody[k] != v {
t.Errorf("payload[%q] = %v, want %v", k, gotBody[k], v)
}
}
if pr.Number != 7 || pr.HTMLURL == "" {
t.Errorf("parsed PR = %+v", pr)
}
})
}
}
// A non-2xx must surface the API's own message rather than a bare status.
func TestEditPRAPIError(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = io.WriteString(w, `{"message":"not found","url":"https://git.unkin.net/api/swagger","errors":null}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
title := "new title"
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
_, err := c.EditPR("unkin/repo", 7, EditOptions{Title: &title})
if err == nil {
t.Fatal("expected error on 404")
}
if !strings.Contains(err.Error(), `"message":"not found"`) {
t.Errorf("error %q should carry the API message", err)
}
}
func TestCreateComment(t *testing.T) {
var gotBody map[string]string
mux := http.NewServeMux()
@@ -544,3 +619,471 @@ func TestAnonymousPollingNeverMints(t *testing.T) {
t.Errorf("sent %d Authorization headers, want none", authHeaders)
}
}
// Anonymous access to something that is not public is not a stale credential:
// nothing was sent to be rejected, so the client must not burn a Vault mint on
// every poll, and the error must say a token was missing rather than expired.
func TestAnonymousAuthFailureNeverMints(t *testing.T) {
requests := 0
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
requests++
w.WriteHeader(http.StatusUnauthorized)
_, _ = io.WriteString(w, `{"message":"invalid username, password or token"}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
refreshes := 0
c := &GiteaClient{BaseURL: srv.URL, HTTP: srv.Client(),
Refresh: func() (string, error) { refreshes++; return "fresh", nil }}
_, err := c.GetPR("unkin/repo", 7)
if err == nil {
t.Fatal("expected an error on an anonymous 401")
}
if !IsNoCredential(err) {
t.Errorf("IsNoCredential(%v) = false, want true", err)
}
if !IsAuthError(err) {
t.Errorf("IsAuthError(%v) = false; an anonymous rejection still ends a watch", err)
}
if refreshes != 0 {
t.Errorf("refreshes = %d, want 0 (nothing was rejected)", refreshes)
}
if requests != 1 {
t.Errorf("requests = %d, want 1 (no replay)", requests)
}
}
// Gitea's bare "Forbidden" is a permission boundary, not an expired token. A
// re-mint cannot grant a permission the identity lacks, so the client must not
// spend one, and the failure must not be reported as an auth expiry.
func TestPermissionDeniedIsNotRemintedOrRetried(t *testing.T) {
requests := 0
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
requests++
w.WriteHeader(http.StatusForbidden)
_, _ = io.WriteString(w, `{"errors":null,"message":"Forbidden","url":"https://git.unkin.net/api/swagger"}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
refreshes := 0
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client(),
Refresh: func() (string, error) { refreshes++; return "fresh", nil }}
_, err := c.GetPR("unkin/repo", 7)
if err == nil {
t.Fatal("expected an error on a 403")
}
if !IsPermissionDenied(err) {
t.Errorf("IsPermissionDenied(%v) = false, want true", err)
}
if IsCredentialRejected(err) {
t.Errorf("a bare Forbidden must not read as a rejected credential")
}
if refreshes != 0 || requests != 1 {
t.Errorf("refreshes = %d, requests = %d, want 0 and 1", refreshes, requests)
}
}
// A 403 that names the token is a credential problem after all (Gitea reports a
// missing scope this way), so it keeps the re-mint-and-replay path.
func TestScopeForbiddenIsRemintedAndRetried(t *testing.T) {
var seen []string
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
tok := strings.TrimPrefix(r.Header.Get("Authorization"), "token ")
seen = append(seen, tok)
if tok != "fresh" {
w.WriteHeader(http.StatusForbidden)
_, _ = io.WriteString(w, `{"message":"token does not have at least one of required scope(s): [read:repository]"}`)
return
}
_, _ = io.WriteString(w, `{"number":7,"state":"open","mergeable":true,"head":{"sha":"cafebabe"}}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
refreshes := 0
c := &GiteaClient{BaseURL: srv.URL, Token: "stale", HTTP: srv.Client(),
Refresh: func() (string, error) { refreshes++; return "fresh", nil }}
pr, err := c.GetPR("unkin/repo", 7)
if err != nil {
t.Fatalf("GetPR after re-mint: %v", err)
}
if pr.Number != 7 || refreshes != 1 {
t.Errorf("PR = %+v, refreshes = %d, want PR 7 and 1 re-mint", pr, refreshes)
}
if len(seen) != 2 || seen[1] != "fresh" {
t.Errorf("tokens seen = %v, want [stale fresh]", seen)
}
}
// A re-mint that hands back an empty token must fail loudly. Replaying with it
// would drop the Authorization header, and on a public repo that anonymous
// replay succeeds — the watch would carry on having quietly lost its identity.
func TestEmptyRemintedTokenFailsInsteadOfGoingAnonymous(t *testing.T) {
var seen []string
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
tok := strings.TrimPrefix(r.Header.Get("Authorization"), "token ")
seen = append(seen, tok)
if tok == "stale" {
w.WriteHeader(http.StatusUnauthorized)
_, _ = io.WriteString(w, `{"message":"invalid username, password or token"}`)
return
}
_, _ = io.WriteString(w, `{"number":7,"state":"open","mergeable":true,"head":{"sha":"cafebabe"}}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "stale", HTTP: srv.Client(),
Refresh: func() (string, error) { return "", nil }}
_, err := c.GetPR("unkin/repo", 7)
if err == nil {
t.Fatal("an empty re-minted token must be an error, not an anonymous retry")
}
if !IsAuthError(err) {
t.Errorf("IsAuthError(%v) = false, want true", err)
}
if len(seen) != 1 {
t.Errorf("requests = %d, want 1 (no anonymous replay)", len(seen))
}
}
// The real-world rotation case end to end: the watch runs happily for several
// polls, then the Vault lease expires and Gitea rejects every token, the fresh
// one included. Watch must end with a named auth failure instead of polling on.
func TestWatchAbortsWhenTokenExpiresMidWatch(t *testing.T) {
polls := 0
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
polls++
if polls > 3 {
w.WriteHeader(http.StatusUnauthorized)
_, _ = io.WriteString(w, `{"message":"invalid username, password or token"}`)
return
}
_, _ = io.WriteString(w, `{"number":7,"state":"open","mergeable":true,"head":{"sha":"cafebabe"}}`)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/commits/cafebabe/status", func(w http.ResponseWriter, r *http.Request) {
_, _ = io.WriteString(w, `{"state":"success"}`)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/7/comments", func(w http.ResponseWriter, r *http.Request) {
_, _ = io.WriteString(w, `[]`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
refreshes := 0
c := &GiteaClient{BaseURL: srv.URL, Token: "t1", HTTP: srv.Client(),
Refresh: func() (string, error) { refreshes++; return "t2", nil }}
ref := PRRef{Owner: "unkin", Repo: "repo", Number: 7}
ticks := make(chan time.Time, 5)
for i := 0; i < 5; i++ {
ticks <- time.Now()
}
close(ticks)
warned := 0
_, err := Watch(c, []PRRef{ref}, "unkin-agent", ticks, nil, func(PRRef, error) { warned++ })
if err == nil {
t.Fatal("Watch returned nil: an expired token must end the watch, not be polled past")
}
if !IsAuthError(err) {
t.Errorf("Watch error = %v, want an auth error", err)
}
if IsNoCredential(err) {
t.Errorf("a rejected token must not be reported as a missing one: %v", err)
}
if refreshes != 1 {
t.Errorf("refreshes = %d, want 1", refreshes)
}
if warned != 0 {
t.Errorf("auth failure logged as a warning %d time(s); it must abort", warned)
}
}
func TestCreateIssueRequestBody(t *testing.T) {
var gotPath, gotMethod, gotAuth string
var gotBody CreateIssueOptions
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/issues", func(w http.ResponseWriter, r *http.Request) {
gotPath, gotMethod = r.URL.Path, r.Method
gotAuth = r.Header.Get("Authorization")
_ = json.NewDecoder(r.Body).Decode(&gotBody)
_, _ = 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: "gitea-abc", HTTP: srv.Client()}
issue, err := c.CreateIssue("unkin/repo", CreateIssueOptions{Title: "T", Body: "B"})
if err != nil {
t.Fatalf("CreateIssue: %v", err)
}
if gotMethod != http.MethodPost || gotPath != "/api/v1/repos/unkin/repo/issues" {
t.Errorf("request = %s %s, want POST /api/v1/repos/unkin/repo/issues", gotMethod, gotPath)
}
if gotAuth != "token gitea-abc" {
t.Errorf("auth header = %q, want 'token gitea-abc'", gotAuth)
}
if gotBody.Title != "T" || gotBody.Body != "B" {
t.Errorf("request body = %+v", gotBody)
}
if issue.Number != 12 || issue.HTMLURL != "https://git.unkin.net/unkin/repo/issues/12" {
t.Errorf("parsed issue = %+v", issue)
}
}
// Filing against a repo that does not exist (or that the token may not see)
// gets Gitea's 404, which must surface as a not-found error carrying the API's
// own message rather than a bare status.
func TestCreateIssueRepoNotFound(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = io.WriteString(w, `{"errors":null,"message":"user redirect does not exist [name: ghost]","url":"https://git.unkin.net/api/swagger"}`)
}))
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
_, err := c.CreateIssue("ghost/repo", CreateIssueOptions{Title: "T"})
if err == nil {
t.Fatal("expected error for a repo that does not exist")
}
if !IsNotFound(err) {
t.Errorf("IsNotFound(%v) = false, want true", err)
}
if !strings.Contains(err.Error(), "user redirect does not exist") {
t.Errorf("error %q should carry the API message", err)
}
}
// Any other non-2xx is a plain API failure: reported, not retried, and not
// mistaken for a missing repo.
func TestCreateIssueAPIError(t *testing.T) {
requests := 0
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/issues", func(w http.ResponseWriter, r *http.Request) {
requests++
w.WriteHeader(http.StatusUnprocessableEntity)
_, _ = io.WriteString(w, `{"errors":null,"message":"Validation Error: title is empty","url":"https://git.unkin.net/api/swagger"}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
_, err := c.CreateIssue("unkin/repo", CreateIssueOptions{Title: "T"})
if err == nil {
t.Fatal("expected error on 422")
}
if IsNotFound(err) {
t.Errorf("a 422 must not read as not-found: %v", err)
}
if !strings.Contains(err.Error(), "Validation Error") {
t.Errorf("error %q should carry the API message", err)
}
if requests != 1 {
t.Errorf("requests = %d, want 1 (a 422 is not retried)", requests)
}
}
// An issue edit sends only the fields it was given, for the same reason a PR
// edit does: Gitea overwrites whatever key it receives.
func TestEditIssueSendsOnlySuppliedFields(t *testing.T) {
title, body := "new title", "new body"
tests := []struct {
name string
opts EditOptions
want map[string]any
}{
{"body only", EditOptions{Body: &body}, map[string]any{"body": "new body"}},
{"title only", EditOptions{Title: &title}, map[string]any{"title": "new title"}},
{"both", EditOptions{Title: &title, Body: &body}, map[string]any{"title": "new title", "body": "new body"}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var gotBody map[string]any
var gotMethod, gotPath string
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/12", func(w http.ResponseWriter, r *http.Request) {
gotMethod, gotPath = r.Method, r.URL.Path
_ = json.NewDecoder(r.Body).Decode(&gotBody)
_, _ = io.WriteString(w, `{"number":12,"title":"new title","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()}
issue, err := c.EditIssue("unkin/repo", 12, tt.opts)
if err != nil {
t.Fatalf("EditIssue: %v", err)
}
if gotMethod != http.MethodPatch {
t.Errorf("method = %s, want PATCH", gotMethod)
}
if gotPath != "/api/v1/repos/unkin/repo/issues/12" {
t.Errorf("path = %q", gotPath)
}
if len(gotBody) != len(tt.want) {
t.Errorf("payload = %v, want exactly the supplied fields %v", gotBody, tt.want)
}
for k, v := range tt.want {
if gotBody[k] != v {
t.Errorf("payload[%q] = %v, want %v", k, gotBody[k], v)
}
}
if issue.Number != 12 || issue.HTMLURL == "" {
t.Errorf("parsed issue = %+v", issue)
}
})
}
}
// 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)
}
}
+31
View File
@@ -109,6 +109,37 @@ func GitRemoteBranchExists(repoDir, remote, branch string) bool {
return err == nil
}
// GitRevParse resolves ref to a full object id in repoDir.
func GitRevParse(repoDir, ref string) (string, error) {
return runGit(repoDir, "rev-parse", ref)
}
// GitAheadCount counts commits reachable from head that upstream does not hold.
func GitAheadCount(repoDir, upstream, head string) (int, error) {
out, err := runGit(repoDir, "rev-list", "--count", upstream+".."+head)
if err != nil {
return 0, err
}
n, err := strconv.Atoi(strings.TrimSpace(out))
if err != nil {
return 0, fmt.Errorf("parse rev-list count %q: %w", out, err)
}
return n, nil
}
// GitMergeFFOnly advances the branch checked out at dir to ref, failing rather
// than writing a merge commit when the move is not a fast-forward.
func GitMergeFFOnly(dir, ref string) error {
_, err := runGit(dir, "merge", "--ff-only", ref)
return err
}
// GitSetUpstream points branch at the remote-tracking ref upstream.
func GitSetUpstream(repoDir, branch, upstream string) error {
_, err := runGit(repoDir, "branch", "--set-upstream-to="+upstream, branch)
return err
}
// GitIsDirty reports whether the checkout at dir has uncommitted or untracked
// changes.
func GitIsDirty(dir string) (bool, error) {
+168 -13
View File
@@ -32,14 +32,61 @@ func IsNotFound(err error) bool {
return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
}
// IsAuthError reports whether err is a Gitea 401/403: the token is expired or
// unauthorised, which retrying the same request cannot fix.
// IsAuthError reports whether err is a Gitea 401/403. Both end a watch: neither
// a rejected credential nor a permission boundary clears itself on a retry.
func IsAuthError(err error) bool {
var apiErr *APIError
return errors.As(err, &apiErr) &&
(apiErr.StatusCode == http.StatusUnauthorized || apiErr.StatusCode == http.StatusForbidden)
}
// ErrNoCredential marks a 401/403 on a request that carried no token at all.
// Anonymous polling of a public repo is supported, so this is not a rejected
// credential: the resource simply is not public and no token was available.
var ErrNoCredential = errors.New("gitea requires authentication and no token was available")
// IsNoCredential reports whether err is an auth failure on an anonymous request.
func IsNoCredential(err error) bool {
return errors.Is(err, ErrNoCredential)
}
// credentialHints are the fragments Gitea puts in a 403 body when the
// credential itself is at fault ("token does not have at least one of required
// scope(s)", "sign in required") rather than the identity's permissions, whose
// body is a bare "Forbidden".
var credentialHints = []string{"token", "sign in", "credential"}
// IsCredentialRejected reports whether err means the credential that was sent
// was refused, which a freshly minted token may fix. A 401 always qualifies.
// Gitea 403s both for a token missing a scope and for an identity that may not
// do this at all, so for a 403 the response body decides.
func IsCredentialRejected(err error) bool {
var apiErr *APIError
if !errors.As(err, &apiErr) {
return false
}
switch apiErr.StatusCode {
case http.StatusUnauthorized:
return true
case http.StatusForbidden:
body := strings.ToLower(apiErr.Body)
for _, hint := range credentialHints {
if strings.Contains(body, hint) {
return true
}
}
}
return false
}
// IsPermissionDenied reports a 403 that names no credential problem: the
// identity is authenticated but not allowed, so re-minting cannot help.
func IsPermissionDenied(err error) bool {
var apiErr *APIError
return errors.As(err, &apiErr) &&
apiErr.StatusCode == http.StatusForbidden && !IsCredentialRejected(err)
}
// GiteaClient talks to the Gitea REST API as the agent user.
type GiteaClient struct {
BaseURL string
@@ -56,8 +103,10 @@ func NewGiteaClient(token string) *GiteaClient {
return &GiteaClient{BaseURL: GiteaURL(), Token: token, HTTP: httpClient, Refresh: RefreshGiteaToken}
}
// do sends the request and, if the token was rejected, re-mints it once and
// replays the request with the fresh token.
// do sends the request and, if the credential it carried was rejected, re-mints
// the token once and replays the request. Anonymous requests and permission
// denials are returned as they are: neither is fixed by a fresh token, and
// re-minting on them would report a stale token as the cause of something else.
func (c *GiteaClient) do(method, path string, body any, out any) error {
var payload []byte
if body != nil {
@@ -67,14 +116,24 @@ func (c *GiteaClient) do(method, path string, body any, out any) error {
}
payload = b
}
anonymous := c.Token == ""
err := c.attempt(method, path, payload, out)
if !IsAuthError(err) || c.Refresh == nil {
if !IsAuthError(err) {
return err
}
if anonymous {
return fmt.Errorf("%w: %w", ErrNoCredential, err)
}
if !IsCredentialRejected(err) || c.Refresh == nil {
return err
}
token, refreshErr := c.Refresh()
if refreshErr != nil {
return fmt.Errorf("%w; re-minting token: %v", err, refreshErr)
}
if token == "" {
return fmt.Errorf("%w; re-minting token yielded an empty token", err)
}
c.Token = token
return c.attempt(method, path, payload, out)
}
@@ -130,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
@@ -214,6 +278,25 @@ func (c *GiteaClient) CreatePR(repoPath string, opts CreatePROptions) (PullReque
return pr, err
}
// EditOptions are the fields an edit may change, for a pull request or an
// issue alike. Pointers so an unset field is omitted from the payload
// entirely, leaving that field as it is. The two fields are not symmetric:
// Gitea only applies a title when it is non-empty, so Title can be set but
// never cleared and a "" title is a silent no-op, while a pointer to "" Body
// really does blank the body.
type EditOptions struct {
Title *string `json:"title,omitempty"`
Body *string `json:"body,omitempty"`
}
// EditPR updates a pull request's title and/or body
// (PATCH /api/v1/repos/{owner}/{repo}/pulls/{index}).
func (c *GiteaClient) EditPR(repoPath string, number int, opts EditOptions) (PullRequest, error) {
var pr PullRequest
err := c.do(http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/pulls/%d", repoPath, number), opts, &pr)
return pr, err
}
// GetPR fetches a single pull request.
func (c *GiteaClient) GetPR(repoPath string, number int) (PullRequest, error) {
var pr PullRequest
@@ -221,6 +304,76 @@ func (c *GiteaClient) GetPR(repoPath string, number int) (PullRequest, error) {
return pr, err
}
// Issue is the subset of Gitea's issue object we track. Gitea numbers issues
// and pull requests in one sequence, so Number is comparable to a PR number.
type Issue struct {
Number int `json:"number"`
State string `json:"state"`
Title string `json:"title"`
HTMLURL string `json:"html_url"`
}
// CreateIssueOptions are the fields for filing an issue.
type CreateIssueOptions struct {
Title string `json:"title"`
Body string `json:"body"`
}
// CreateIssue files an issue (POST /api/v1/repos/{owner}/{repo}/issues).
func (c *GiteaClient) CreateIssue(repoPath string, opts CreateIssueOptions) (Issue, error) {
var issue Issue
err := c.do(http.MethodPost, "/api/v1/repos/"+repoPath+"/issues", opts, &issue)
return issue, err
}
// EditIssue updates an issue's title and/or body
// (PATCH /api/v1/repos/{owner}/{repo}/issues/{index}).
func (c *GiteaClient) EditIssue(repoPath string, number int, opts EditOptions) (Issue, error) {
var issue Issue
err := c.do(http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/issues/%d", repoPath, number), opts, &issue)
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"`
@@ -228,8 +381,10 @@ type Comment struct {
Body string `json:"body"`
}
// CreateComment posts a comment on the PR's issue thread
// (POST /api/v1/repos/{owner}/{repo}/issues/{n}/comments).
// CreateComment posts a comment on an issue thread
// (POST /api/v1/repos/{owner}/{repo}/issues/{n}/comments). Gitea backs a pull
// request with an issue of the same number, so this is the single path for
// both.
func (c *GiteaClient) CreateComment(repoPath string, number int, body string) (Comment, error) {
var cm Comment
payload := map[string]string{"body": body}
+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)
}
}