Compare commits
20 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 5488a1ddc6 | |||
| ebdd25f725 | |||
| 5928233a97 | |||
| 7ffa123e3d | |||
| 91509cb9b2 | |||
| 15c527ee56 | |||
| 27e48ac45e | |||
| 7baa194c52 | |||
| 54c1d868d3 | |||
| 6988be9ff3 | |||
| 0cf41409f1 | |||
| 9de9dffab1 | |||
| d77607c4f0 | |||
| ff8ac5ea2d | |||
| 81ae4041cd | |||
| 2610ea5c09 | |||
| 1b90e60aeb | |||
| 90ce747a61 | |||
| 7510187243 | |||
| 3de9d35699 |
+10
-3
@@ -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
|
||||
|
||||
@@ -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 and edit pull requests, and post PR comments, as
|
||||
`unkin-agent` (fixes the "tea posts as Ben" attribution problem).
|
||||
Subcommands: `pr create`, `pr comment`, `pr edit`, `whoami`.
|
||||
- **`agentpr`** — create and edit pull requests and issues, close and reopen
|
||||
issues, and post comments on either, as `unkin-agent` (fixes the "tea posts
|
||||
as Ben" attribution problem). Subcommands: `pr create`, `pr comment`,
|
||||
`pr edit`, `issue create`, `issue comment`, `issue edit`, `issue close`,
|
||||
`issue reopen`, `whoami`.
|
||||
- **`watchpr`** — poll one or more PRs and exit when a tracked PR changes
|
||||
meaningfully: it merges/closes, gets a new non-agent comment, its CI fails,
|
||||
or it loses mergeability. Benign transitions (CI pending→success, the agent's
|
||||
@@ -28,7 +30,7 @@ parsing, watch-state comparison, git worktree helpers).
|
||||
## Structure
|
||||
|
||||
```
|
||||
cmd/agentpr/main.go # agentpr CLI (pr create / pr comment / pr edit / whoami)
|
||||
cmd/agentpr/main.go # agentpr CLI (pr + issue create/comment/edit, issue close/reopen, whoami)
|
||||
cmd/watchpr/main.go # watchpr CLI (poll + meaningful-change exit)
|
||||
cmd/agentws/main.go # agentws CLI (new / list / rm / clean / token / credential)
|
||||
cmd/agentws/prune.go # agentws prune (classify worktrees, remove the safe ones)
|
||||
@@ -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/edit/get, comments, status, whoami)
|
||||
gitea.go # Gitea REST client (PR/issue create/edit/get, issue state, comments, status, whoami)
|
||||
parse.go # owner/repo#N and owner/repo parsing
|
||||
watch.go # PRState snapshot + MeaningfulChange comparison
|
||||
git.go # git worktree/clone/fetch helpers (os/exec, no go-git)
|
||||
@@ -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,45 @@ wrapped per stage (login / read denied / write denied) via `ErrVaultDenied`.
|
||||
half-finished (`rebase-merge`, `MERGE_HEAD`, `CHERRY_PICK_HEAD`, …). A directory
|
||||
whose backing repo is gone is deleted outright, but only ever inside the
|
||||
worktree root.
|
||||
- `watchpr` alerts on changes, not on conditions: the baseline snapshot is a real
|
||||
read, so a PR that is already conflicted or already CI-failing when watching
|
||||
starts is polled on rather than reported. That suppression is printed with the
|
||||
baseline, since a silent watcher and a watcher with nothing to say look alike.
|
||||
The baseline goes to stderr in both modes — a JSON record under `--json` —
|
||||
because automation is what most needs it and stdout carries the result.
|
||||
- Mergeability is the one `watchpr` rule needing a run of observations
|
||||
(`prWatch`). Gitea 1.26 sends `mergeable` as a plain bool — always present,
|
||||
never null — and sends `false` both for a real conflict and while it recomputes
|
||||
the merge base after a push, which is exactly when an agent starts a watch. So
|
||||
`false` counts only once this watch has seen a merge computation start (the PR
|
||||
was mergeable, or `head.sha`/`base.sha` moved), and then only once the run of
|
||||
`false`s has spanned `conflictWindow` (2m); a mergeable poll, an unknown one or
|
||||
a failed poll all break the run. The debounce is wall-clock, measured from the
|
||||
tick that fired each poll, because what it outlasts is the recompute and
|
||||
`--interval` spans seconds to hours. The window starts at the later of the
|
||||
run's first observation and the arm, which is what keeps arming and the run
|
||||
independent: movement arms without resetting the run — `base.sha` is the base
|
||||
branch tip as of the response, so it moves for every open PR whenever main does
|
||||
and a reset there could never complete on a busy repo — while an arming poll
|
||||
still cannot confirm a run it played no part in. Re-arming an already-armed
|
||||
watch is a no-op, so a base moving under every poll advances the window once
|
||||
and never again. Both times are pointers because the zero `time.Time` is a
|
||||
legal clock value and cannot also mean "no run". `Mergeability` stays tri-state
|
||||
for what the bool cannot carry: an absent/null flag from some other Gitea, and
|
||||
a state no successful poll ever filled in. Head and base SHAs are read only to
|
||||
arm — a push never alerts. The residue: a conflict landed by the push just
|
||||
before the watch began is unreported while the commits stay put, since Gitea's
|
||||
payload has no field separating it from a check in flight (`merge_base` is the
|
||||
true merge base and does not move on recheck). The commits rarely stay put —
|
||||
a merge to the base branch moves `base.sha` under every open PR — but that
|
||||
move only arms; the two minutes it then has to outlast are the recompute the
|
||||
move started, not the falses before it.
|
||||
- CI "combined status" comes from `/commits/{sha}/status`; an empty head SHA
|
||||
yields an empty state without an API call.
|
||||
- Gitea backs every PR with an issue of the same number and serves comments from
|
||||
`/issues/{n}/comments`, so `agentpr pr comment` and `agentpr issue comment`
|
||||
are one implementation under two flag names (`--pr` / `--issue`).
|
||||
- `issue close`/`issue reopen` read the issue before the PATCH: Gitea answers a
|
||||
no-op state change with 200, so without the read an already-closed issue would
|
||||
report success. There is no `pr close`: closing a pull request is a human's
|
||||
call, not an agent's.
|
||||
|
||||
@@ -6,8 +6,8 @@ token from Vault, so automated PRs, comments and pushes are attributed to the
|
||||
agent — not to whoever happens to run the command. Set `AGENT_LOGIN` to act as a
|
||||
different agent identity.
|
||||
|
||||
- **`agentpr`** — create and edit pull requests, and 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
|
||||
@@ -56,6 +56,24 @@ 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
|
||||
```
|
||||
@@ -67,7 +85,38 @@ Non-zero exit on any API error.
|
||||
Poll PRs and exit (reporting what changed) when a tracked PR **merges/closes**,
|
||||
gets a **new comment from someone other than the agent**, its **CI fails**
|
||||
(failure/error), or it **loses mergeability** (a conflict appears). Benign
|
||||
transitions — CI `pending`→`success`, the agent's own comments — are ignored.
|
||||
transitions — CI `pending`→`success`, the agent's own comments and pushes — are
|
||||
ignored, and so is any condition the PR was already in at the baseline: a PR
|
||||
that is already conflicted or already failing when watching starts keeps being
|
||||
watched, and the baseline line — written to stderr, as a JSON record under
|
||||
`--json` — says which conditions it is staying silent about.
|
||||
|
||||
Gitea reports `mergeable: false` both for a real conflict and while it
|
||||
recomputes the merge base after a push, so a conflict is only reported once the
|
||||
watch has seen a merge computation start (the PR was mergeable, or its head or
|
||||
base commit moved) **and** the non-mergeable polls have **then** run unbroken for
|
||||
two minutes. The window runs from whichever came later, so the poll that starts
|
||||
the merge computation never confirms a run of falses that predates it. The
|
||||
debounce is a duration, not a poll count, because what it has to outlast is
|
||||
Gitea's recompute and `--interval` ranges from seconds to hours.
|
||||
|
||||
So a conflict introduced by the push immediately before the watch started is not
|
||||
reported while the commits stay put: nothing in Gitea's payload separates it from
|
||||
a merge check still in flight, and the baseline line is the only notice of it.
|
||||
That is a narrower gap than it looks, because `base.sha` is the base branch's
|
||||
tip — it moves for every open PR whenever anything merges to the base branch, so
|
||||
the commits rarely stay put for long. The first such move arms the rule, and is
|
||||
itself silent: the window then runs from that arming poll rather than from the
|
||||
falses that predate it, so what is eventually reported is a conflict that
|
||||
outlasted the merge computation the move started.
|
||||
|
||||
Arming is a one-way latch. Moves after that one — and every move seen by a watch
|
||||
that was already armed at the baseline — neither re-arm nor restart the window,
|
||||
because a restart on every `base.sha` move could never complete on a busy base
|
||||
branch. That is the cost side of the same trade: on a busy base branch a
|
||||
conflict can be confirmed while the newest merge recompute is less than two
|
||||
minutes old. The window guarantees that the run of falses outlasted *a* merge
|
||||
computation this watch saw start, not that it outlasted the most recent one.
|
||||
|
||||
```bash
|
||||
# Watch until something meaningful happens (default interval 60s)
|
||||
@@ -87,6 +136,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
|
||||
@@ -97,10 +165,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)
|
||||
@@ -124,6 +192,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
|
||||
|
||||
+178
-29
@@ -6,6 +6,11 @@
|
||||
// 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
|
||||
|
||||
@@ -34,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
|
||||
}
|
||||
|
||||
@@ -59,7 +64,22 @@ func newPRCmd() *cobra.Command {
|
||||
Use: "pr",
|
||||
Short: "Create and edit PRs, and post PR comments",
|
||||
}
|
||||
cmd.AddCommand(newPRCreateCmd(), newPRCommentCmd(), newPREditCmd())
|
||||
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
|
||||
}
|
||||
|
||||
@@ -104,20 +124,24 @@ 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 pr <= 0 {
|
||||
return fmt.Errorf("--pr must be a positive PR number")
|
||||
if number <= 0 {
|
||||
return fmt.Errorf("--%s must be a positive %s number", numFlag, noun)
|
||||
}
|
||||
if body == "" {
|
||||
return fmt.Errorf("--body is required")
|
||||
@@ -126,20 +150,20 @@ func newPRCommentCmd() *cobra.Command {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
cm, err := c.CreateComment(owner+"/"+name, pr, body)
|
||||
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, pr)
|
||||
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(&pr, "pr", 0, "PR number (required)")
|
||||
f.IntVar(&number, numFlag, 0, noun+" number (required)")
|
||||
f.StringVar(&body, "body", "", "Comment body (required)")
|
||||
_ = cmd.MarkFlagRequired("repo")
|
||||
_ = cmd.MarkFlagRequired("pr")
|
||||
_ = cmd.MarkFlagRequired(numFlag)
|
||||
_ = cmd.MarkFlagRequired("body")
|
||||
return cmd
|
||||
}
|
||||
@@ -159,22 +183,9 @@ func newPREditCmd() *cobra.Command {
|
||||
if pr <= 0 {
|
||||
return fmt.Errorf("--pr must be a positive PR number")
|
||||
}
|
||||
// Only the flags actually given are sent: omitting --title must
|
||||
// leave the title as it is, not blank it.
|
||||
var opts agent.EditPROptions
|
||||
if cmd.Flags().Changed("title") {
|
||||
// Gitea ignores an empty title, so sending one would report
|
||||
// success while changing nothing.
|
||||
if title == "" {
|
||||
return 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 fmt.Errorf("at least one of --title or --body is required")
|
||||
opts, err := editOptions(cmd, title, body)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
c, err := client()
|
||||
if err != nil {
|
||||
@@ -198,6 +209,144 @@ func newPREditCmd() *cobra.Command {
|
||||
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
|
||||
}
|
||||
|
||||
func newWhoamiCmd() *cobra.Command {
|
||||
return &cobra.Command{
|
||||
Use: "whoami",
|
||||
|
||||
@@ -4,6 +4,8 @@ import (
|
||||
"io"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/cobra"
|
||||
)
|
||||
|
||||
// A malformed --repo must fail the command (so main exits non-zero). ParseRepo
|
||||
@@ -49,3 +51,154 @@ func TestPREditRejectsEmptyTitle(t *testing.T) {
|
||||
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
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
+122
-21
@@ -1,8 +1,19 @@
|
||||
// Command watchpr polls one or more Gitea pull requests and exits when a
|
||||
// tracked PR changes in a way worth alerting on: it merges or closes, gets a
|
||||
// new comment from someone other than the agent, its CI fails, or it loses
|
||||
// mergeability. Benign transitions (CI pending→success, the agent's own
|
||||
// comments) are ignored.
|
||||
// mergeability after the baseline. Benign transitions (CI pending→success, the
|
||||
// agent's own pushes and comments) are ignored, and so is any condition that was
|
||||
// already true at the baseline -- which watchpr prints on stderr (as a JSON
|
||||
// record under --json), so a run started against a conflicted or CI-red PR says
|
||||
// so. A conflict introduced by the push immediately before the watch started is
|
||||
// not reported while the commits stay put, because nothing in Gitea's payload
|
||||
// separates it from a merge check still in flight; the baseline line is the only
|
||||
// notice of it. The first later move of the head or base arms the rule without
|
||||
// alerting, and the window then runs from that arm rather than from the falses
|
||||
// that predate it. Moves after that neither re-arm nor restart the window -- a
|
||||
// restart on every base move could never complete on a busy base branch -- so a
|
||||
// conflict can be confirmed while the newest merge recompute is less than two
|
||||
// minutes old.
|
||||
//
|
||||
// watchpr owner/repo#12 owner/repo:15
|
||||
// watchpr --once --json owner/repo#12
|
||||
@@ -13,7 +24,9 @@ package main
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"git.unkin.net/unkin/agent-tools/internal/agent"
|
||||
@@ -43,7 +56,8 @@ func newRootCmd() *cobra.Command {
|
||||
Short: "Poll Gitea PRs and exit when one changes meaningfully.",
|
||||
Long: "watchpr polls each PR every --interval and exits (reporting what changed)\n" +
|
||||
"when a PR merges/closes, gets a new non-agent comment, its CI fails, or it\n" +
|
||||
"loses mergeability. Accepts refs as owner/repo#N or owner/repo:N.",
|
||||
"loses mergeability after the baseline. Conditions already true at the\n" +
|
||||
"baseline are printed, not alerted on. Refs take owner/repo#N or owner/repo:N.",
|
||||
Version: version,
|
||||
Args: cobra.ArbitraryArgs,
|
||||
SilenceUsage: true,
|
||||
@@ -63,7 +77,7 @@ func newRootCmd() *cobra.Command {
|
||||
}
|
||||
refs = append(refs, ref)
|
||||
}
|
||||
c := clientFor()
|
||||
c := clientFor(jsonMode)
|
||||
if once {
|
||||
return runOnce(c, refs, jsonMode)
|
||||
}
|
||||
@@ -89,15 +103,35 @@ func newRootCmd() *cobra.Command {
|
||||
// clientFor builds the Gitea client. Watching public repos works anonymously,
|
||||
// so an unavailable token is a warning, not a failure; a poll that is actually
|
||||
// rejected re-mints then.
|
||||
func clientFor() *agent.GiteaClient {
|
||||
func clientFor(jsonMode bool) *agent.GiteaClient {
|
||||
token, err := agent.GiteaToken()
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "warning: no Gitea token (%v); polling anonymously\n", err)
|
||||
warn(os.Stderr, jsonMode, "no Gitea token (%v); polling anonymously", err)
|
||||
token = ""
|
||||
}
|
||||
return agent.NewGiteaClient(token)
|
||||
}
|
||||
|
||||
// warnRecord is the --json form of a warning. Under --json every notice watchpr
|
||||
// itself writes to stderr -- warnings and the baseline -- is an NDJSON record,
|
||||
// so a caller parsing them line by line never has to guess which shape a line
|
||||
// is. A terminal failure is the one exception: SilenceErrors stays off, so
|
||||
// cobra prints it as a plain "Error: ..." line and the exit status is non-zero.
|
||||
type warnRecord struct {
|
||||
Warning string `json:"warning"`
|
||||
}
|
||||
|
||||
// warn writes a non-fatal notice to stderr in whichever form the caller asked
|
||||
// for.
|
||||
func warn(w io.Writer, jsonMode bool, format string, args ...any) {
|
||||
msg := fmt.Sprintf(format, args...)
|
||||
if jsonMode {
|
||||
_ = json.NewEncoder(w).Encode(warnRecord{Warning: msg})
|
||||
return
|
||||
}
|
||||
_, _ = fmt.Fprintf(w, "warning: %s\n", msg)
|
||||
}
|
||||
|
||||
// runOnce fetches and prints the current state of each PR, then exits 0.
|
||||
func runOnce(c *agent.GiteaClient, refs []agent.PRRef, jsonMode bool) error {
|
||||
login := agent.AgentLogin()
|
||||
@@ -105,7 +139,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,29 +160,65 @@ func runWatch(c *agent.GiteaClient, refs []agent.PRRef, interval time.Duration,
|
||||
ticker := time.NewTicker(interval)
|
||||
defer ticker.Stop()
|
||||
|
||||
onBaseline := func() {
|
||||
if !jsonMode {
|
||||
fmt.Fprintf(os.Stderr, "watching %d PR(s) every %s; baseline established\n", len(refs), interval)
|
||||
}
|
||||
onBaseline := func(states []agent.PRState) {
|
||||
emitBaselines(os.Stderr, states, interval, jsonMode)
|
||||
}
|
||||
onError := func(ref agent.PRRef, err error) {
|
||||
fmt.Fprintf(os.Stderr, "warning: polling %s: %v\n", ref.String(), err)
|
||||
warn(os.Stderr, jsonMode, "polling %s: %v", ref.String(), err)
|
||||
}
|
||||
|
||||
res, err := agent.Watch(c, refs, login, ticker.C, onBaseline, onError)
|
||||
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 +234,39 @@ func report(key, reason string, st agent.PRState, jsonMode bool) {
|
||||
}
|
||||
|
||||
func printState(st agent.PRState) {
|
||||
fmt.Printf("%s state=%s merged=%t mergeable=%t ci=%s head=%s comments(non-agent)=%d\n",
|
||||
st.Ref.String(), st.State, st.Merged, st.Mergeable, ciOrNone(st.CIStatus), shortSHA(st.HeadSHA), st.NonAgentComments)
|
||||
fmt.Println(stateLine(st))
|
||||
}
|
||||
|
||||
func stateLine(st agent.PRState) string {
|
||||
return fmt.Sprintf("%s state=%s merged=%t mergeable=%s ci=%s head=%s base=%s comments(non-agent)=%d",
|
||||
st.Ref.String(), st.State, st.Merged, st.Mergeable, ciOrNone(st.CIStatus),
|
||||
shortSHA(st.HeadSHA), shortSHA(st.BaseSHA), st.NonAgentComments)
|
||||
}
|
||||
|
||||
// baselineLine describes the state a watch started from, naming the conditions
|
||||
// it will deliberately stay silent about: watchpr alerts on changes, so a PR
|
||||
// that is already conflicted or already CI-red produces no alert for either, and
|
||||
// that suppression has to be visible to whoever started the watch.
|
||||
func baselineLine(st agent.PRState) string {
|
||||
line := " baseline " + stateLine(st)
|
||||
if s := suppressedAtBaseline(st); s != "" {
|
||||
return line + " -- " + s + "; not alerting on a pre-existing condition"
|
||||
}
|
||||
return line
|
||||
}
|
||||
|
||||
func suppressedAtBaseline(st agent.PRState) string {
|
||||
var conds []string
|
||||
switch st.Mergeable {
|
||||
case agent.MergeNo:
|
||||
conds = append(conds, "already non-mergeable (a real conflict, or Gitea still recomputing)")
|
||||
case agent.MergeUnknown:
|
||||
conds = append(conds, "mergeability unknown (the conflict rule is disarmed until Gitea reports mergeable or the commits move)")
|
||||
}
|
||||
if st.CIStatus == "failure" || st.CIStatus == "error" {
|
||||
conds = append(conds, "CI already "+st.CIStatus)
|
||||
}
|
||||
return strings.Join(conds, ", ")
|
||||
}
|
||||
|
||||
func ciOrNone(s string) string {
|
||||
|
||||
@@ -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,202 @@ 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())
|
||||
}
|
||||
}
|
||||
|
||||
// Under --json stderr is the baseline and warning channel, so every notice
|
||||
// watchpr writes there has to be one shape: a caller parsing it line by line
|
||||
// must never meet a bare `warning:` line between two NDJSON records. Cobra's
|
||||
// terminal `Error: ...` line is not covered here -- SilenceErrors stays off, so
|
||||
// it is plain text on stderr alongside a non-zero exit.
|
||||
func TestJSONModeWarningsAndBaselineAreRecords(t *testing.T) {
|
||||
st := agent.PRState{
|
||||
Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7},
|
||||
State: "open",
|
||||
Mergeable: agent.MergeNo,
|
||||
}
|
||||
var buf bytes.Buffer
|
||||
warn(&buf, true, "no Gitea token (%v); polling anonymously", errors.New("vault refused"))
|
||||
emitBaselines(&buf, []agent.PRState{st}, 30*time.Second, true)
|
||||
warn(&buf, true, "polling %s: %v", st.Ref.String(), errors.New("HTTP 502"))
|
||||
|
||||
lines := strings.Split(strings.TrimSpace(buf.String()), "\n")
|
||||
if len(lines) != 3 {
|
||||
t.Fatalf("stderr = %q, want 3 records", buf.String())
|
||||
}
|
||||
for _, line := range lines {
|
||||
var rec map[string]any
|
||||
if err := json.Unmarshal([]byte(line), &rec); err != nil {
|
||||
t.Errorf("stderr line %q is not a JSON record: %v", line, err)
|
||||
}
|
||||
}
|
||||
var first warnRecord
|
||||
if err := json.Unmarshal([]byte(lines[0]), &first); err != nil || !strings.Contains(first.Warning, "vault refused") {
|
||||
t.Errorf("first record = %q, want the token warning", lines[0])
|
||||
}
|
||||
|
||||
buf.Reset()
|
||||
warn(&buf, false, "polling %s: %v", st.Ref.String(), errors.New("HTTP 502"))
|
||||
if got := buf.String(); !strings.HasPrefix(got, "warning: ") {
|
||||
t.Errorf("plain-mode warning = %q, want the warning: prefix", got)
|
||||
}
|
||||
}
|
||||
|
||||
// An unknown mergeability disarms the conflict rule exactly as a non-mergeable
|
||||
// baseline does, so the baseline has to name it too.
|
||||
func TestBaselineNamesAnUnknownMergeability(t *testing.T) {
|
||||
st := agent.PRState{
|
||||
Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7},
|
||||
State: "open",
|
||||
Mergeable: agent.MergeUnknown,
|
||||
CIStatus: "success",
|
||||
}
|
||||
if got := suppressedAtBaseline(st); !strings.Contains(got, "mergeability unknown") {
|
||||
t.Errorf("suppressed = %q, want it to name the unknown mergeability", got)
|
||||
}
|
||||
if got := baselineLine(st); !strings.Contains(got, "not alerting") {
|
||||
t.Errorf("baselineLine = %q, want the suppression note", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// fakeVault serves the AppRole login and the gitea creds secret at credsPath
|
||||
@@ -104,13 +105,13 @@ func TestEditPRSendsOnlySuppliedFields(t *testing.T) {
|
||||
title, body, empty := "new title", "new body", ""
|
||||
tests := []struct {
|
||||
name string
|
||||
opts EditPROptions
|
||||
opts EditOptions
|
||||
want map[string]any
|
||||
}{
|
||||
{"body only", EditPROptions{Body: &body}, map[string]any{"body": "new body"}},
|
||||
{"title only", EditPROptions{Title: &title}, map[string]any{"title": "new title"}},
|
||||
{"both", EditPROptions{Title: &title, Body: &body}, map[string]any{"title": "new title", "body": "new body"}},
|
||||
{"explicit empty body is sent", EditPROptions{Body: &empty}, map[string]any{"body": ""}},
|
||||
{"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) {
|
||||
@@ -163,7 +164,7 @@ func TestEditPRAPIError(t *testing.T) {
|
||||
|
||||
title := "new title"
|
||||
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
|
||||
_, err := c.EditPR("unkin/repo", 7, EditPROptions{Title: &title})
|
||||
_, err := c.EditPR("unkin/repo", 7, EditOptions{Title: &title})
|
||||
if err == nil {
|
||||
t.Fatal("expected error on 404")
|
||||
}
|
||||
@@ -618,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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
+157
-20
@@ -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,19 +278,20 @@ func (c *GiteaClient) CreatePR(repoPath string, opts CreatePROptions) (PullReque
|
||||
return pr, err
|
||||
}
|
||||
|
||||
// EditPROptions are the fields an edit may change. 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 EditPROptions struct {
|
||||
// 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 EditPROptions) (PullRequest, error) {
|
||||
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
|
||||
@@ -239,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"`
|
||||
@@ -246,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}
|
||||
|
||||
+190
-28
@@ -3,6 +3,7 @@ package agent
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -16,17 +17,65 @@ func IsPRGone(err error) bool {
|
||||
return errors.Is(err, errPRGone)
|
||||
}
|
||||
|
||||
// Mergeability is Gitea's mergeable flag. Gitea 1.26 always sends a plain bool,
|
||||
// and sends false both for a real conflict and while it recomputes the merge
|
||||
// base after a push, so false on its own decides nothing (prWatch resolves it).
|
||||
// Unknown covers what the bool cannot carry: an absent or null flag from another
|
||||
// Gitea build, and a snapshot no successful poll ever filled in.
|
||||
type Mergeability int
|
||||
|
||||
const (
|
||||
MergeUnknown Mergeability = iota
|
||||
MergeYes
|
||||
MergeNo
|
||||
)
|
||||
|
||||
func (m Mergeability) String() string {
|
||||
switch m {
|
||||
case MergeYes:
|
||||
return "true"
|
||||
case MergeNo:
|
||||
return "false"
|
||||
}
|
||||
return "unknown"
|
||||
}
|
||||
|
||||
func (m Mergeability) MarshalJSON() ([]byte, error) {
|
||||
switch m {
|
||||
case MergeYes:
|
||||
return []byte("true"), nil
|
||||
case MergeNo:
|
||||
return []byte("false"), nil
|
||||
}
|
||||
return []byte("null"), nil
|
||||
}
|
||||
|
||||
func (m *Mergeability) UnmarshalJSON(b []byte) error {
|
||||
switch strings.TrimSpace(string(b)) {
|
||||
case "true":
|
||||
*m = MergeYes
|
||||
case "false":
|
||||
*m = MergeNo
|
||||
case "null":
|
||||
*m = MergeUnknown
|
||||
default:
|
||||
return fmt.Errorf("mergeable: unexpected value %s", b)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// PRState is a point-in-time snapshot of the PR attributes watchpr tracks.
|
||||
type PRState struct {
|
||||
Ref PRRef `json:"ref"`
|
||||
State string `json:"state"` // open / closed
|
||||
Merged bool `json:"merged"`
|
||||
HeadSHA string `json:"head_sha"`
|
||||
Mergeable bool `json:"mergeable"`
|
||||
CIStatus string `json:"ci_status"` // success / pending / failure / error / ""
|
||||
NonAgentComments int `json:"non_agent_comments"`
|
||||
Title string `json:"title"`
|
||||
URL string `json:"url"`
|
||||
Ref PRRef `json:"ref"`
|
||||
State string `json:"state"` // open / closed
|
||||
Merged bool `json:"merged"`
|
||||
HeadSHA string `json:"head_sha"`
|
||||
BaseSHA string `json:"base_sha"`
|
||||
Mergeable Mergeability `json:"mergeable"`
|
||||
CIStatus string `json:"ci_status"` // success / pending / failure / error / ""
|
||||
NonAgentComments int `json:"non_agent_comments"`
|
||||
Title string `json:"title"`
|
||||
URL string `json:"url"`
|
||||
}
|
||||
|
||||
// FetchState builds a PRState for the given ref. agentLogin's comments are
|
||||
@@ -55,6 +104,7 @@ func FetchState(c *GiteaClient, ref PRRef, agentLogin string) (PRState, error) {
|
||||
State: pr.State,
|
||||
Merged: pr.Merged,
|
||||
HeadSHA: pr.Head.Sha,
|
||||
BaseSHA: pr.Base.Sha,
|
||||
Mergeable: pr.Mergeable,
|
||||
CIStatus: ci,
|
||||
NonAgentComments: countNonAgentComments(comments, agentLogin),
|
||||
@@ -95,6 +145,121 @@ func terminalState(st PRState) (bool, string) {
|
||||
return false, ""
|
||||
}
|
||||
|
||||
// conflictWindow is how long an unbroken run of non-mergeable polls must span
|
||||
// before it is reported as a conflict. What the run has to outlast is Gitea's
|
||||
// merge recompute, which is a duration, so counting polls cannot express it:
|
||||
// --interval varies from seconds to hours between callers. Recomputes have been
|
||||
// observed finishing in ~5-20s with no bound on the tail, so this is set well
|
||||
// clear of that; the cost of it being too long is only a later conflict alert,
|
||||
// while too short is a false one.
|
||||
const conflictWindow = 2 * time.Minute
|
||||
|
||||
// prWatch tracks one PR across polls, because mergeability needs more memory
|
||||
// than the previous snapshot. Gitea reports mergeable=false while it recomputes
|
||||
// the merge base after a push, so a false is only trusted once this watch has
|
||||
// seen a merge computation start: the PR was mergeable at some point, or its
|
||||
// head or base SHA moved. A bare false inherited from the baseline says nothing
|
||||
// -- it is equally a conflict the operator is already waiting on and a recompute
|
||||
// in flight -- so it arms nothing. Arming and the run of falses are independent:
|
||||
// movement only arms and never resets the run, because a base branch that moves
|
||||
// under the PR on every push to main would otherwise restart it forever. The
|
||||
// window is instead measured from the later of the run's start and the arm, so
|
||||
// an arming poll confirms nothing it has not itself outlasted.
|
||||
type prWatch struct {
|
||||
prev PRState
|
||||
armed bool
|
||||
// armedAt is when the rule armed mid-watch; nil while disarmed, and nil when
|
||||
// the baseline armed it, since then there is no transition to measure from.
|
||||
armedAt *time.Time
|
||||
// conflictSince is when the current unbroken run of non-mergeable polls
|
||||
// began; nil when no run is in progress.
|
||||
conflictSince *time.Time
|
||||
}
|
||||
|
||||
func newPRWatch(baseline PRState) *prWatch {
|
||||
return &prWatch{prev: baseline, armed: baseline.Mergeable == MergeYes}
|
||||
}
|
||||
|
||||
// arm records the disarmed->armed transition and when it happened. Re-arming is
|
||||
// a no-op, so a base branch moving under every poll advances nothing.
|
||||
func (w *prWatch) arm(now time.Time) {
|
||||
if w.armed {
|
||||
return
|
||||
}
|
||||
w.armed = true
|
||||
at := now
|
||||
w.armedAt = &at
|
||||
}
|
||||
|
||||
// mergeInputsChanged reports whether the commits Gitea merges have moved, which
|
||||
// starts a fresh merge computation whose result is attributable to this watch.
|
||||
func mergeInputsChanged(prev, cur PRState) bool {
|
||||
return cur.HeadSHA != prev.HeadSHA || cur.BaseSHA != prev.BaseSHA
|
||||
}
|
||||
|
||||
// track folds one snapshot's mergeability into the run of observations. The
|
||||
// observations that confirm a conflict must be adjacent, so anything but
|
||||
// another non-mergeable one breaks the run.
|
||||
func (w *prWatch) track(st PRState, now time.Time) {
|
||||
switch st.Mergeable {
|
||||
case MergeNo:
|
||||
if w.conflictSince == nil {
|
||||
at := now
|
||||
w.conflictSince = &at
|
||||
}
|
||||
case MergeYes:
|
||||
w.arm(now)
|
||||
w.conflictSince = nil
|
||||
default:
|
||||
w.conflictSince = nil
|
||||
}
|
||||
}
|
||||
|
||||
// missed records a poll that never produced a snapshot; the run of adjacent
|
||||
// non-mergeable observations does not survive the gap.
|
||||
func (w *prWatch) missed() {
|
||||
w.conflictSince = nil
|
||||
}
|
||||
|
||||
// windowStart is the later of the run's first observation and the arm, so the
|
||||
// window always covers observations this watch can attribute a merge
|
||||
// computation to. A baseline arm records no time and leaves the run governing.
|
||||
func (w *prWatch) windowStart() *time.Time {
|
||||
if w.conflictSince == nil {
|
||||
return nil
|
||||
}
|
||||
if w.armedAt != nil && w.armedAt.After(*w.conflictSince) {
|
||||
return w.armedAt
|
||||
}
|
||||
return w.conflictSince
|
||||
}
|
||||
|
||||
// confirmed reports whether the run of non-mergeable observations has spanned
|
||||
// the recompute window since it started counting, so neither a lone
|
||||
// non-mergeable poll nor the poll that armed the rule confirms anything.
|
||||
func (w *prWatch) confirmed(now time.Time) bool {
|
||||
start := w.windowStart()
|
||||
return start != nil && now.Sub(*start) >= conflictWindow
|
||||
}
|
||||
|
||||
// observe folds in the newest snapshot, taken at now, and reports whether the
|
||||
// watch should end.
|
||||
func (w *prWatch) observe(cur PRState, now time.Time) (bool, string) {
|
||||
changed, reason := MeaningfulChange(w.prev, cur)
|
||||
if mergeInputsChanged(w.prev, cur) {
|
||||
w.arm(now)
|
||||
}
|
||||
w.prev = cur
|
||||
w.track(cur, now)
|
||||
if changed {
|
||||
return true, reason
|
||||
}
|
||||
if w.armed && w.confirmed(now) && cur.State == "open" {
|
||||
return true, "PR lost mergeability (conflict)"
|
||||
}
|
||||
return false, ""
|
||||
}
|
||||
|
||||
// MaxPollFailures is how many consecutive failed polls of the same PR are
|
||||
// tolerated before Watch gives up. The abort fires on the 20th failed tick, so
|
||||
// at watchpr's default 60s interval a watch rides out ~19 minutes of failure.
|
||||
@@ -109,10 +274,11 @@ const MaxPollFailures = 20
|
||||
// is gone, renamed, or no longer visible), and MaxPollFailures consecutive
|
||||
// failures of one PR all abort, because a watcher that sees nothing must not
|
||||
// look healthy.
|
||||
// onBaseline, if set, fires once after all baselines are captured and before the
|
||||
// first tick.
|
||||
func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Time, onBaseline func(), onError func(PRRef, error)) (WatchResult, error) {
|
||||
prev := make(map[string]PRState, len(refs))
|
||||
// onBaseline, if set, receives every captured baseline once, before the first
|
||||
// tick, so a caller can show what state the watch started from.
|
||||
func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Time, onBaseline func([]PRState), onError func(PRRef, error)) (WatchResult, error) {
|
||||
watches := make(map[string]*prWatch, len(refs))
|
||||
baselines := make([]PRState, 0, len(refs))
|
||||
for _, ref := range refs {
|
||||
st, err := f.FetchState(ref, agentLogin)
|
||||
if err != nil {
|
||||
@@ -121,13 +287,16 @@ func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Ti
|
||||
if terminal, reason := terminalState(st); terminal {
|
||||
return WatchResult{Ref: ref, Reason: reason, State: st}, nil
|
||||
}
|
||||
prev[ref.String()] = st
|
||||
watches[ref.String()] = newPRWatch(st)
|
||||
baselines = append(baselines, st)
|
||||
}
|
||||
if onBaseline != nil {
|
||||
onBaseline()
|
||||
onBaseline(baselines)
|
||||
}
|
||||
fails := make(map[string]int, len(refs))
|
||||
for range ticks {
|
||||
// The tick carries the time it fired, which is the clock the conflict
|
||||
// window is measured on.
|
||||
for now := range ticks {
|
||||
for _, ref := range refs {
|
||||
key := ref.String()
|
||||
cur, err := f.FetchState(ref, agentLogin)
|
||||
@@ -136,6 +305,7 @@ func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Ti
|
||||
return WatchResult{}, fmt.Errorf("polling %s: %w", key, err)
|
||||
}
|
||||
fails[key]++
|
||||
watches[key].missed()
|
||||
if onError != nil {
|
||||
onError(ref, err)
|
||||
}
|
||||
@@ -145,10 +315,9 @@ func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Ti
|
||||
continue
|
||||
}
|
||||
fails[key] = 0
|
||||
if changed, reason := MeaningfulChange(prev[key], cur); changed {
|
||||
if changed, reason := watches[key].observe(cur, now); changed {
|
||||
return WatchResult{Ref: ref, Reason: reason, State: cur}, nil
|
||||
}
|
||||
prev[key] = cur
|
||||
}
|
||||
}
|
||||
return WatchResult{}, nil
|
||||
@@ -172,15 +341,15 @@ func isFailedCI(state string) bool {
|
||||
|
||||
// MeaningfulChange compares a previous state to the current one and reports
|
||||
// whether a change warrants alerting the operator, with a human-readable
|
||||
// reason. Benign transitions (CI pending→success, the agent's own comments, an
|
||||
// unchanged snapshot) return false.
|
||||
// reason. Benign transitions (CI pending→success, the agent's own comments, a
|
||||
// new head or base commit, an unchanged snapshot) return false. Mergeability is
|
||||
// not decided here: it takes a whole run of observations, which prWatch keeps.
|
||||
//
|
||||
// Alerting conditions:
|
||||
// - the PR merged
|
||||
// - the PR closed without merging
|
||||
// - a new comment from someone other than the agent
|
||||
// - CI transitioned into failure/error
|
||||
// - the PR lost mergeability (a conflict appeared) for two consecutive polls
|
||||
func MeaningfulChange(prev, cur PRState) (bool, string) {
|
||||
if !prev.Merged && cur.Merged {
|
||||
return true, "PR merged"
|
||||
@@ -195,12 +364,5 @@ func MeaningfulChange(prev, cur PRState) (bool, string) {
|
||||
if isFailedCI(cur.CIStatus) && !isFailedCI(prev.CIStatus) {
|
||||
return true, "CI failed (" + cur.CIStatus + ")"
|
||||
}
|
||||
// Gitea computes mergeability asynchronously, so a PR can briefly report
|
||||
// mergeable=false right after a push. Require the loss to persist across two
|
||||
// consecutive polls (both prev and cur false, still open) before treating it
|
||||
// as a real conflict; a single false poll is debounced.
|
||||
if !prev.Mergeable && !cur.Mergeable && cur.State == "open" {
|
||||
return true, "PR lost mergeability (conflict)"
|
||||
}
|
||||
return false, ""
|
||||
}
|
||||
|
||||
+612
-24
@@ -1,6 +1,7 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
@@ -17,7 +18,8 @@ func base() PRState {
|
||||
State: "open",
|
||||
Merged: false,
|
||||
HeadSHA: "abc123",
|
||||
Mergeable: true,
|
||||
BaseSHA: "base000",
|
||||
Mergeable: MergeYes,
|
||||
CIStatus: "pending",
|
||||
NonAgentComments: 0,
|
||||
}
|
||||
@@ -66,25 +68,11 @@ func TestMeaningfulChange(t *testing.T) {
|
||||
wantChange: true,
|
||||
},
|
||||
{
|
||||
// A single mergeable=false poll is debounced: Gitea often reports
|
||||
// this transiently right after a push.
|
||||
name: "mergeable true to false for one poll is benign",
|
||||
mutate: func(s *PRState) { s.Mergeable = false },
|
||||
wantChange: false,
|
||||
},
|
||||
{
|
||||
// mergeable=false persisting into a second consecutive poll is a
|
||||
// real conflict and alerts.
|
||||
name: "mergeable false persisting a second poll alerts",
|
||||
mutatePrev: func(s *PRState) { s.Mergeable = false },
|
||||
mutate: func(s *PRState) { s.Mergeable = false },
|
||||
wantChange: true,
|
||||
},
|
||||
{
|
||||
// mergeable recovered (false then true) must not alert.
|
||||
name: "mergeable recovered false to true is benign",
|
||||
mutatePrev: func(s *PRState) { s.Mergeable = false },
|
||||
mutate: func(s *PRState) {},
|
||||
// Mergeability takes a run of observations, so no pair of snapshots
|
||||
// decides it here; prWatch owns that rule.
|
||||
name: "mergeable false pair alone is not a pairwise change",
|
||||
mutatePrev: func(s *PRState) { s.Mergeable = MergeNo },
|
||||
mutate: func(s *PRState) { s.Mergeable = MergeNo },
|
||||
wantChange: false,
|
||||
},
|
||||
{
|
||||
@@ -222,16 +210,16 @@ func TestWatchDetectsMergeAfterBaseline(t *testing.T) {
|
||||
merged.Merged = true
|
||||
f := &fakeFetcher{states: []PRState{open, merged}} // baseline open, then merged
|
||||
|
||||
baselineFired := false
|
||||
var baselines []PRState
|
||||
ticks := make(chan time.Time, 1)
|
||||
ticks <- time.Now()
|
||||
res, err := Watch(f, []PRRef{open.Ref}, "unkin-agent",
|
||||
ticks, func() { baselineFired = true }, nil)
|
||||
ticks, func(sts []PRState) { baselines = sts }, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if !baselineFired {
|
||||
t.Errorf("onBaseline should fire for an open baseline")
|
||||
if len(baselines) != 1 || baselines[0].Ref != open.Ref {
|
||||
t.Errorf("onBaseline received %v, want the one open baseline", baselines)
|
||||
}
|
||||
if res.Reason != "PR merged" {
|
||||
t.Errorf("reason = %q, want %q", res.Reason, "PR merged")
|
||||
@@ -801,3 +789,603 @@ func TestWatchAnonymousKeepsPolling(t *testing.T) {
|
||||
t.Errorf("fetch calls = %d, want 3 (baseline + two polls)", f.calls)
|
||||
}
|
||||
}
|
||||
|
||||
// drainableTicks returns a channel holding n ticks and already closed, so Watch
|
||||
// polls exactly n times and then returns instead of blocking. The ticks are
|
||||
// spaced far past conflictWindow, so a run of non-mergeable polls confirms on
|
||||
// its second observation; spacedTicks drives the intervals where it must not.
|
||||
func drainableTicks(n int) <-chan time.Time {
|
||||
return spacedTicks(n, 10*time.Minute)
|
||||
}
|
||||
|
||||
// spacedTicks is drainableTicks with the poll interval named. The tick carries
|
||||
// the time Watch measures the conflict window on, which makes every wall-clock
|
||||
// assertion in these tests exact and instant.
|
||||
func spacedTicks(n int, interval time.Duration) <-chan time.Time {
|
||||
start := time.Date(2026, 9, 24, 12, 0, 0, 0, time.UTC)
|
||||
ticks := make(chan time.Time, n)
|
||||
for i := 1; i <= n; i++ {
|
||||
ticks <- start.Add(time.Duration(i) * interval)
|
||||
}
|
||||
close(ticks)
|
||||
return ticks
|
||||
}
|
||||
|
||||
// The production bug: a PR that was already conflicted (and already CI-failing)
|
||||
// when watching began must not be reported as having just changed. That state is
|
||||
// what the watcher is waiting to see resolved, so the loop keeps polling.
|
||||
func TestWatchIgnoresBaselineConflictAndFailure(t *testing.T) {
|
||||
stuck := base()
|
||||
stuck.Mergeable = MergeNo
|
||||
stuck.CIStatus = "failure"
|
||||
f := &fakeFetcher{states: []PRState{stuck}}
|
||||
|
||||
res, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", drainableTicks(5), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "" {
|
||||
t.Fatalf("Watch ended with %q; a conflict/failure predating the watch is not a change", res.Reason)
|
||||
}
|
||||
if f.calls != 6 {
|
||||
t.Errorf("fetch calls = %d, want 6 (baseline + 5 polls)", f.calls)
|
||||
}
|
||||
}
|
||||
|
||||
// Mergeability lost after the baseline still alerts, on the second consecutive
|
||||
// conflicted poll.
|
||||
func TestWatchDetectsConflictAfterBaseline(t *testing.T) {
|
||||
ok := base()
|
||||
conflicted := base()
|
||||
conflicted.Mergeable = MergeNo
|
||||
f := &fakeFetcher{states: []PRState{ok, conflicted, conflicted}}
|
||||
|
||||
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "PR lost mergeability (conflict)" {
|
||||
t.Errorf("reason = %q, want the mergeability loss", res.Reason)
|
||||
}
|
||||
if f.calls != 3 {
|
||||
t.Errorf("fetch calls = %d, want 3 (baseline + the two conflicted polls)", f.calls)
|
||||
}
|
||||
}
|
||||
|
||||
// CI that goes green→red during the watch alerts.
|
||||
func TestWatchDetectsCIFailureAfterBaseline(t *testing.T) {
|
||||
ok := base()
|
||||
ok.CIStatus = "pending"
|
||||
green := base()
|
||||
green.CIStatus = "success"
|
||||
red := base()
|
||||
red.CIStatus = "failure"
|
||||
f := &fakeFetcher{states: []PRState{ok, green, red}}
|
||||
|
||||
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "CI failed (failure)" {
|
||||
t.Errorf("reason = %q, want the CI failure (pending→success must pass silently)", res.Reason)
|
||||
}
|
||||
if f.calls != 3 {
|
||||
t.Errorf("fetch calls = %d, want 3 (the success poll must not end the watch)", f.calls)
|
||||
}
|
||||
}
|
||||
|
||||
// The agent's own pushes and comments must not end a watch; a comment from
|
||||
// anyone else must.
|
||||
func TestWatchIgnoresAgentActivity(t *testing.T) {
|
||||
start := base()
|
||||
pushed := base()
|
||||
pushed.HeadSHA = "def456" // the agent pushed a fix; non-agent comments unchanged
|
||||
commented := pushed
|
||||
commented.NonAgentComments = 1
|
||||
f := &fakeFetcher{states: []PRState{start, pushed, pushed, commented}}
|
||||
|
||||
res, err := Watch(f, []PRRef{start.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "new comment from a non-agent user" {
|
||||
t.Errorf("reason = %q, want the non-agent comment", res.Reason)
|
||||
}
|
||||
if f.calls != 4 {
|
||||
t.Errorf("fetch calls = %d, want 4 (the agent's push and comment must not end the watch)", f.calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRWatchMergeability(t *testing.T) {
|
||||
snap := func(m Mergeability) PRState {
|
||||
s := base()
|
||||
s.Mergeable = m
|
||||
return s
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
baseline Mergeability
|
||||
polls []Mergeability
|
||||
wantPoll int // 1-based poll that ends the watch; 0 for none
|
||||
}{
|
||||
{"conflicted before the watch never alerts", MergeNo, []Mergeability{MergeNo, MergeNo, MergeNo}, 0},
|
||||
{"loss after a mergeable baseline alerts on the second poll", MergeYes, []Mergeability{MergeNo, MergeNo}, 2},
|
||||
{"a lone conflicted poll is debounced", MergeYes, []Mergeability{MergeNo, MergeYes, MergeNo}, 0},
|
||||
{"unknown at baseline does not arm the rule", MergeUnknown, []Mergeability{MergeNo, MergeNo, MergeNo}, 0},
|
||||
{"unknown at baseline then a real loss alerts", MergeUnknown, []Mergeability{MergeYes, MergeNo, MergeNo}, 3},
|
||||
{"a baseline conflict resolved then lost again alerts", MergeNo, []Mergeability{MergeYes, MergeNo, MergeNo}, 3},
|
||||
{"unknown between conflicted polls breaks the run", MergeYes, []Mergeability{MergeNo, MergeUnknown, MergeNo}, 0},
|
||||
{"the run restarts after an unknown", MergeYes, []Mergeability{MergeNo, MergeUnknown, MergeNo, MergeNo}, 4},
|
||||
{"unknown alone is never a conflict", MergeYes, []Mergeability{MergeUnknown, MergeUnknown, MergeUnknown}, 0},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
w := newPRWatch(snap(tt.baseline))
|
||||
start := time.Date(2026, 9, 24, 12, 0, 0, 0, time.UTC)
|
||||
got := 0
|
||||
for i, m := range tt.polls {
|
||||
changed, reason := w.observe(snap(m), start.Add(time.Duration(i+1)*10*time.Minute))
|
||||
if !changed {
|
||||
continue
|
||||
}
|
||||
if reason != "PR lost mergeability (conflict)" {
|
||||
t.Fatalf("poll %d ended the watch with %q, want a mergeability loss", i+1, reason)
|
||||
}
|
||||
got = i + 1
|
||||
break
|
||||
}
|
||||
if got != tt.wantPoll {
|
||||
t.Errorf("alerted on poll %d, want %d", got, tt.wantPoll)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Gitea 1.26 always sends mergeable as a plain bool, so unknown is reserved for
|
||||
// what that bool cannot carry: an absent or null flag must decode as unknown
|
||||
// rather than as a conflict, and unknown must encode back as null.
|
||||
func TestMergeabilityDecoding(t *testing.T) {
|
||||
tests := map[string]Mergeability{
|
||||
`{"number":7}`: MergeUnknown,
|
||||
`{"number":7,"mergeable":null}`: MergeUnknown,
|
||||
`{"number":7,"mergeable":true}`: MergeYes,
|
||||
`{"number":7,"mergeable":false}`: MergeNo,
|
||||
}
|
||||
for body, want := range tests {
|
||||
var pr PullRequest
|
||||
if err := json.Unmarshal([]byte(body), &pr); err != nil {
|
||||
t.Fatalf("Unmarshal(%s): %v", body, err)
|
||||
}
|
||||
if pr.Mergeable != want {
|
||||
t.Errorf("Unmarshal(%s) mergeable = %v, want %v", body, pr.Mergeable, want)
|
||||
}
|
||||
}
|
||||
out, err := json.Marshal(PRState{Mergeable: MergeUnknown})
|
||||
if err != nil {
|
||||
t.Fatalf("Marshal: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(out), `"mergeable":null`) {
|
||||
t.Errorf("unknown mergeability encoded as %s, want null", out)
|
||||
}
|
||||
}
|
||||
|
||||
// The inverse of the bug this PR fixed: demanding a mergeable observation to arm
|
||||
// the rule left a PR that was non-mergeable at baseline silent forever, even
|
||||
// after a push gave Gitea a fresh merge computation to answer for. A head that
|
||||
// moves during the watch arms the rule, so the conflict the new head keeps
|
||||
// reporting is attributable to this watch and is reported.
|
||||
func TestWatchAlertsWhenAPushArmsABaselineConflict(t *testing.T) {
|
||||
baseline := base()
|
||||
baseline.Mergeable = MergeNo // still recomputing; the push is not visible yet
|
||||
|
||||
pushed := base()
|
||||
pushed.Mergeable = MergeNo
|
||||
pushed.HeadSHA = "def456"
|
||||
|
||||
f := &fakeFetcher{states: []PRState{baseline, pushed, pushed, pushed}}
|
||||
res, err := Watch(f, []PRRef{baseline.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "PR lost mergeability (conflict)" {
|
||||
t.Fatalf("reason = %q, want the mergeability loss; a conflict must not be unreportable because the baseline caught the recompute", res.Reason)
|
||||
}
|
||||
if f.calls != 3 {
|
||||
t.Errorf("fetch calls = %d, want 3 (baseline + the two conflicted polls on the new head)", f.calls)
|
||||
}
|
||||
}
|
||||
|
||||
// A base-branch move restarts the same merge computation, so a conflict that
|
||||
// only becomes visible once someone merges into main alerts too.
|
||||
func TestWatchAlertsWhenBaseMovedIntoAConflict(t *testing.T) {
|
||||
baseline := base()
|
||||
baseline.Mergeable = MergeNo
|
||||
|
||||
moved := base()
|
||||
moved.Mergeable = MergeNo
|
||||
moved.BaseSHA = "base111"
|
||||
|
||||
f := &fakeFetcher{states: []PRState{baseline, moved, moved}}
|
||||
res, err := Watch(f, []PRRef{baseline.Ref}, "unkin-agent", drainableTicks(3), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "PR lost mergeability (conflict)" {
|
||||
t.Errorf("reason = %q, want the mergeability loss", res.Reason)
|
||||
}
|
||||
}
|
||||
|
||||
// The fix this PR exists for: a conflict predating the watch, on a head and base
|
||||
// that never move, is the condition the operator is already waiting on and must
|
||||
// stay silent however long the watch runs.
|
||||
func TestWatchNeverAlertsOnAStableBaselineConflict(t *testing.T) {
|
||||
stuck := base()
|
||||
stuck.Mergeable = MergeNo
|
||||
f := &fakeFetcher{states: []PRState{stuck}}
|
||||
|
||||
res, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", drainableTicks(100), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "" {
|
||||
t.Fatalf("Watch ended with %q after 100 unchanged polls; a conflict that predates the watch is not a change", res.Reason)
|
||||
}
|
||||
}
|
||||
|
||||
// Head and base movement arm the conflict rule but are not themselves alerts: a
|
||||
// push, or a base that moves under the PR, must not end a watch.
|
||||
func TestWatchArmingMovementDoesNotAlert(t *testing.T) {
|
||||
start := base()
|
||||
pushed := base()
|
||||
pushed.HeadSHA = "def456"
|
||||
rebased := base()
|
||||
rebased.HeadSHA = "789abc"
|
||||
rebased.BaseSHA = "base111"
|
||||
f := &fakeFetcher{states: []PRState{start, pushed, rebased}}
|
||||
|
||||
res, err := Watch(f, []PRRef{start.Ref}, "unkin-agent", drainableTicks(5), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "" {
|
||||
t.Fatalf("Watch ended with %q; a new head or base is not a change worth alerting on", res.Reason)
|
||||
}
|
||||
}
|
||||
|
||||
// The polls that confirm a conflict must be adjacent. A failed poll produces no
|
||||
// snapshot, so the run cannot span it and two non-adjacent falses do not fire.
|
||||
func TestWatchConflictRunDoesNotSpanAFailedPoll(t *testing.T) {
|
||||
ok := base()
|
||||
conflicted := base()
|
||||
conflicted.Mergeable = MergeNo
|
||||
f := &fakeFetcher{
|
||||
states: []PRState{ok, conflicted, conflicted, conflicted, conflicted},
|
||||
errs: []error{nil, nil, errors.New("HTTP 502"), nil, nil},
|
||||
}
|
||||
|
||||
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", drainableTicks(4), nil, func(PRRef, error) {})
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "PR lost mergeability (conflict)" {
|
||||
t.Fatalf("reason = %q, want the mergeability loss on the two adjacent polls", res.Reason)
|
||||
}
|
||||
if f.calls != 5 {
|
||||
t.Errorf("fetch calls = %d, want 5: the falses either side of the failed poll are not a run", f.calls)
|
||||
}
|
||||
}
|
||||
|
||||
// perRefFetcher scripts a separate sequence per ref, so a multi-ref watch can be
|
||||
// driven with each PR doing something different.
|
||||
type perRefFetcher struct {
|
||||
states map[string][]PRState
|
||||
calls map[string]int
|
||||
}
|
||||
|
||||
func (f *perRefFetcher) FetchState(ref PRRef, _ string) (PRState, error) {
|
||||
key := ref.String()
|
||||
seq := f.states[key]
|
||||
i := f.calls[key]
|
||||
if i >= len(seq) {
|
||||
i = len(seq) - 1
|
||||
}
|
||||
f.calls[key]++
|
||||
return seq[i], nil
|
||||
}
|
||||
|
||||
// Each ref keeps its own run of observations: one PR's mergeability, pushes and
|
||||
// resolutions must neither arm nor disarm another's conflict rule.
|
||||
func TestWatchTracksRefsIndependently(t *testing.T) {
|
||||
refA := PRRef{Owner: "unkin", Repo: "repo", Number: 1}
|
||||
refB := PRRef{Owner: "unkin", Repo: "repo", Number: 2}
|
||||
snap := func(ref PRRef, m Mergeability, head string) PRState {
|
||||
s := base()
|
||||
s.Ref = ref
|
||||
s.Mergeable = m
|
||||
s.HeadSHA = head
|
||||
return s
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
a, b []PRState
|
||||
want string // reason, "" for no alert
|
||||
}{
|
||||
{
|
||||
name: "a mergeable neighbour does not arm a baseline conflict",
|
||||
a: []PRState{snap(refA, MergeYes, "aaa")},
|
||||
b: []PRState{snap(refB, MergeNo, "bbb")},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "a neighbour's push does not arm a baseline conflict",
|
||||
a: []PRState{snap(refA, MergeYes, "aaa"), snap(refA, MergeYes, "aa2"), snap(refA, MergeYes, "aa3")},
|
||||
b: []PRState{snap(refB, MergeNo, "bbb")},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "a neighbour's mergeable polls do not clear another's run",
|
||||
a: []PRState{snap(refA, MergeYes, "aaa")},
|
||||
b: []PRState{snap(refB, MergeYes, "bbb"), snap(refB, MergeNo, "bbb"), snap(refB, MergeNo, "bbb")},
|
||||
want: "PR lost mergeability (conflict)",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
f := &perRefFetcher{
|
||||
states: map[string][]PRState{refA.String(): tt.a, refB.String(): tt.b},
|
||||
calls: map[string]int{},
|
||||
}
|
||||
res, err := Watch(f, []PRRef{refA, refB}, "unkin-agent", drainableTicks(6), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != tt.want {
|
||||
t.Fatalf("reason = %q, want %q", res.Reason, tt.want)
|
||||
}
|
||||
if tt.want != "" && res.Ref != refB {
|
||||
t.Errorf("alert names %s, want %s", res.Ref, refB)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A watch that will deliberately stay silent about a pre-existing conflict has
|
||||
// to hand its caller the baseline it is staying silent about.
|
||||
func TestWatchReportsBaselineStates(t *testing.T) {
|
||||
stuck := base()
|
||||
stuck.Mergeable = MergeNo
|
||||
stuck.CIStatus = "failure"
|
||||
f := &fakeFetcher{states: []PRState{stuck}}
|
||||
|
||||
var got []PRState
|
||||
if _, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", drainableTicks(1),
|
||||
func(sts []PRState) { got = sts }, nil); err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("onBaseline received %d state(s), want 1", len(got))
|
||||
}
|
||||
if got[0].Mergeable != MergeNo || got[0].CIStatus != "failure" {
|
||||
t.Errorf("baseline = %+v, want the conflicted, CI-red snapshot the watch started from", got[0])
|
||||
}
|
||||
}
|
||||
|
||||
// The residual gap, pinned so it is a decision rather than an accident: the
|
||||
// agent pushes, the push conflicts, watchpr starts inside Gitea's recompute and
|
||||
// nothing moves again. Every poll answers false and none of them is
|
||||
// attributable to this watch, so no alert is ever sent -- the baseline line is
|
||||
// the only notice. Gitea's payload carries no field that separates this from a
|
||||
// merge check still running.
|
||||
func TestWatchNeverAlertsOnAConflictLandedByThePushBeforeTheWatch(t *testing.T) {
|
||||
pushed := base()
|
||||
pushed.Mergeable = MergeNo
|
||||
pushed.HeadSHA = "def456"
|
||||
f := &fakeFetcher{states: []PRState{pushed}}
|
||||
|
||||
res, err := Watch(f, []PRRef{pushed.Ref}, "unkin-agent", drainableTicks(50), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "" {
|
||||
t.Fatalf("Watch ended with %q; nothing distinguishes this conflict from a merge check in flight, so it must stay silent", res.Reason)
|
||||
}
|
||||
}
|
||||
|
||||
// The whole mergeability rule as a sequence table, driven through Watch at the
|
||||
// intervals that decide it. The debounce is wall-clock, so the same sequence of
|
||||
// snapshots must alert or stay silent according to how far apart the polls are.
|
||||
func TestWatchConflictSequences(t *testing.T) {
|
||||
snap := func(m Mergeability, head, bse string) PRState {
|
||||
st := base()
|
||||
st.Mergeable = m
|
||||
st.HeadSHA = head
|
||||
st.BaseSHA = bse
|
||||
return st
|
||||
}
|
||||
repeat := func(st PRState, n int) []PRState {
|
||||
out := make([]PRState, n)
|
||||
for i := range out {
|
||||
out[i] = st
|
||||
}
|
||||
return out
|
||||
}
|
||||
movingBase := func(m Mergeability, n int) []PRState {
|
||||
out := make([]PRState, n)
|
||||
for i := range out {
|
||||
out[i] = snap(m, "h1", fmt.Sprintf("b%d", i))
|
||||
}
|
||||
return out
|
||||
}
|
||||
const (
|
||||
quick = 5 * time.Second // well inside conflictWindow
|
||||
normal = 60 * time.Second // watchpr's default
|
||||
relaxed = 10 * time.Minute // past conflictWindow in a single gap
|
||||
)
|
||||
const conflict = "PR lost mergeability (conflict)"
|
||||
|
||||
yes := snap(MergeYes, "h1", "b1")
|
||||
no := snap(MergeNo, "h1", "b1")
|
||||
pushedNo := snap(MergeNo, "h2", "b1")
|
||||
movedNo := snap(MergeNo, "h1", "b2")
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
interval time.Duration
|
||||
baseline PRState
|
||||
polls []PRState
|
||||
errs []error
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "a push then the recompute's falses is not a conflict",
|
||||
interval: normal, baseline: yes, polls: repeat(pushedNo, 2), want: "",
|
||||
},
|
||||
{
|
||||
name: "a fast poller rides out the whole recompute after a push",
|
||||
interval: quick, baseline: yes, polls: repeat(pushedNo, 20), want: "",
|
||||
},
|
||||
{
|
||||
name: "a base move then the recompute's falses is not a conflict",
|
||||
interval: normal, baseline: no, polls: repeat(movedNo, 2), want: "",
|
||||
},
|
||||
{
|
||||
name: "a base moving under every poll still confirms a conflict",
|
||||
interval: normal, baseline: yes, polls: movingBase(MergeNo, 20), want: conflict,
|
||||
},
|
||||
{
|
||||
name: "a base move does not confirm a run it took no part in",
|
||||
interval: normal, baseline: no, polls: []PRState{no, no, no, movedNo}, want: "",
|
||||
},
|
||||
{
|
||||
name: "a push does not confirm a run it took no part in",
|
||||
interval: normal, baseline: no, polls: []PRState{no, no, no, pushedNo}, want: "",
|
||||
},
|
||||
{
|
||||
name: "an armed run confirms a window after the arm, not before",
|
||||
interval: normal, baseline: no,
|
||||
polls: []PRState{no, no, no, movedNo, movedNo, movedNo}, want: conflict,
|
||||
},
|
||||
{
|
||||
name: "a conflict that predates the watch stays silent forever",
|
||||
interval: relaxed, baseline: no, polls: repeat(no, 50), want: "",
|
||||
},
|
||||
{
|
||||
name: "a sustained loss after a mergeable baseline alerts",
|
||||
interval: normal, baseline: yes, polls: repeat(no, 3), want: conflict,
|
||||
},
|
||||
{
|
||||
name: "a mergeable poll breaks the run however long it ran",
|
||||
interval: relaxed, baseline: yes, polls: []PRState{no, yes, no}, want: "",
|
||||
},
|
||||
{
|
||||
name: "an unknown poll breaks the run however long it ran",
|
||||
interval: relaxed, baseline: yes, polls: []PRState{no, snap(MergeUnknown, "h1", "b1"), no}, want: "",
|
||||
},
|
||||
{
|
||||
name: "a baseline conflict that clears and returns alerts",
|
||||
interval: relaxed, baseline: no, polls: []PRState{yes, no, no}, want: conflict,
|
||||
},
|
||||
{
|
||||
name: "a failed poll breaks the run",
|
||||
interval: relaxed, baseline: yes, polls: []PRState{no, no, no},
|
||||
errs: []error{nil, nil, errors.New("HTTP 502"), nil}, want: "",
|
||||
},
|
||||
{
|
||||
name: "the run restarts after a failed poll and still confirms",
|
||||
interval: relaxed, baseline: yes, polls: []PRState{no, no, no, no},
|
||||
errs: []error{nil, nil, errors.New("HTTP 502"), nil, nil}, want: conflict,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
f := &fakeFetcher{states: append([]PRState{tt.baseline}, tt.polls...), errs: tt.errs}
|
||||
res, err := Watch(f, []PRRef{tt.baseline.Ref}, "unkin-agent",
|
||||
spacedTicks(len(tt.polls), tt.interval), nil, func(PRRef, error) {})
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != tt.want {
|
||||
t.Fatalf("reason = %q, want %q", res.Reason, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Two refs watched together keep separate runs and separate clocks: a PR being
|
||||
// pushed to must neither delay nor trigger the conflict its neighbour is really
|
||||
// in, and the alert must name the conflicted one.
|
||||
func TestWatchConflictIsolatedFromANeighboursPushes(t *testing.T) {
|
||||
conflicted := PRRef{Owner: "unkin", Repo: "repo", Number: 1}
|
||||
pushing := PRRef{Owner: "unkin", Repo: "repo", Number: 2}
|
||||
snap := func(ref PRRef, m Mergeability, head string) PRState {
|
||||
st := base()
|
||||
st.Ref = ref
|
||||
st.Mergeable = m
|
||||
st.HeadSHA = head
|
||||
return st
|
||||
}
|
||||
f := &perRefFetcher{
|
||||
states: map[string][]PRState{
|
||||
conflicted.String(): {snap(conflicted, MergeYes, "a1"), snap(conflicted, MergeNo, "a1")},
|
||||
pushing.String(): {snap(pushing, MergeYes, "b1"), snap(pushing, MergeYes, "b2"), snap(pushing, MergeYes, "b3")},
|
||||
},
|
||||
calls: map[string]int{},
|
||||
}
|
||||
|
||||
res, err := Watch(f, []PRRef{conflicted, pushing}, "unkin-agent", drainableTicks(4), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "PR lost mergeability (conflict)" {
|
||||
t.Fatalf("reason = %q, want the conflict on %s", res.Reason, conflicted)
|
||||
}
|
||||
if res.Ref != conflicted {
|
||||
t.Errorf("alert names %s, want %s", res.Ref, conflicted)
|
||||
}
|
||||
}
|
||||
|
||||
// The window is measured from the arm, not from the run the arming poll walked
|
||||
// in on: a base branch that moves under a long-conflicted PR arms the rule and
|
||||
// then has to outlast the recompute itself before anything is reported.
|
||||
func TestWatchConflictWindowStartsAtTheArm(t *testing.T) {
|
||||
stuck := base()
|
||||
stuck.Mergeable = MergeNo
|
||||
moved := stuck
|
||||
moved.BaseSHA = "base111"
|
||||
|
||||
f := &fakeFetcher{states: []PRState{stuck, stuck, stuck, stuck, moved, moved, moved}}
|
||||
res, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", spacedTicks(6, time.Minute), nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "PR lost mergeability (conflict)" {
|
||||
t.Fatalf("reason = %q, want the mergeability loss two minutes after the base moved", res.Reason)
|
||||
}
|
||||
if f.calls != 7 {
|
||||
t.Errorf("fetch calls = %d, want 7: the window runs from the arming poll, not from the run it inherited", f.calls)
|
||||
}
|
||||
}
|
||||
|
||||
// The zero time is a legal clock value, so it must not double as the "no run in
|
||||
// progress" sentinel: a caller whose ticks start at the zero time gets the same
|
||||
// debounce as any other.
|
||||
func TestWatchConflictWindowCountsFromTheZeroTime(t *testing.T) {
|
||||
ok := base()
|
||||
conflicted := base()
|
||||
conflicted.Mergeable = MergeNo
|
||||
|
||||
ticks := make(chan time.Time, 2)
|
||||
ticks <- time.Time{}
|
||||
ticks <- time.Time{}.Add(10 * time.Minute)
|
||||
close(ticks)
|
||||
|
||||
f := &fakeFetcher{states: []PRState{ok, conflicted, conflicted}}
|
||||
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", ticks, nil, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Watch: %v", err)
|
||||
}
|
||||
if res.Reason != "PR lost mergeability (conflict)" {
|
||||
t.Fatalf("reason = %q, want the mergeability loss; a run starting at the zero time still counts", res.Reason)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user