29 Commits

Author SHA1 Message Date
benvin 0cf41409f1 Merge pull request 'agentpr: close and reopen issues' (#19) from benvin/agentpr-issue-state into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #19
2026-09-20 23:08:34 +10:00
unkin-agent 9de9dffab1 agentpr: close and reopen issues
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
- add issue close and issue reopen subcommands
- add GetIssue and SetIssueState to the Gitea client
- fail when the issue is already in the requested state
- document the new subcommands in README.md and AGENTS.md
2026-09-20 23:06:15 +10:00
benvin d77607c4f0 Merge pull request 'Name the cause when a watch stops on an auth failure' (#18) from benvin/watchpr-auth-failure into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #18
2026-09-19 17:04:18 +10:00
unkin-agent ff8ac5ea2d Merge main into watchpr auth failure branch
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
Keep both sides' additions to client_test.go and adopt main's
EditPROptions -> EditOptions rename.
2026-09-19 16:40:44 +10:00
benvin 81ae4041cd Merge pull request 'agentws: check out --branch from origin when it exists' (#17) from benvin/agentws-branch-checkout into main
Reviewed-on: #17
2026-09-19 16:36:45 +10:00
benvin 2610ea5c09 Merge pull request 'Add agentpr issue create/comment/edit' (#16) from benvin/agentpr-issue into main
Reviewed-on: #16
2026-09-19 16:28:33 +10:00
unkin-agent 1b90e60aeb Run go test once per module in pre-commit
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
The go-unit-tests hook caps every package at 30s and re-runs the whole module
once per file batch, so the git-fixture tests in cmd/agentws timed out as soon
as they grew.

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

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

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

- add `issue create`, `issue comment` and `issue edit`, mirroring the pr
  command group's flags, output and errors
- add CreateIssue/EditIssue to the Gitea client, sharing EditOptions with
  EditPR
- build both comment commands from one constructor so pr and issue
  comments cannot drift off Gitea's shared endpoint
- cover create, edit, a missing repo, an API error and missing flags
2026-09-19 16:11:42 +10:00
benvin 72adebbf8b Merge pull request 'Add agentpr pr edit subcommand' (#15) from benvin/agentpr-pr-edit into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #15
2026-09-19 13:48:44 +10:00
unkin-agent 3f990c841d Mock the 404 body Gitea really returns
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
2026-09-19 12:44:00 +10:00
unkin-agent 78d83b7a61 Reject an empty pr edit title Gitea would ignore
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline failed
2026-09-19 12:38:12 +10:00
unkin-agent 7bc4082cb0 Add agentpr pr edit subcommand
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
Update a PR's title and/or body as the agent user, sending only the
fields supplied.
2026-09-19 12:32:07 +10:00
benvin cdced6536e Merge pull request 'Find every stale worktree, not just the managed ones' (#14) from benvin/worktree-gc into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #14
2026-09-12 00:46:39 +10:00
unkin-agent 72a8923c3d Cover the keep gate for unmanaged worktrees
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
--include-unmanaged lifts only the location gate, and nothing proved the keep
gate still held behind it.

- assert a dirty hand-made worktree survives --include-unmanaged alone
2026-09-12 00:40:12 +10:00
unkin-agent 62aeaf063b Keep worktrees git could not read, never delete them
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was canceled
Any git error on a directory under the worktree root was classified orphan,
and orphan deletes the directory outright, so a transient failure reading the
source root became data loss on a plain `agentws prune --yes`.

- prove a backing repo gone by stat before calling a directory an orphan
- classify an unexplained git failure as keep, naming the error
- refuse to remove a worktree whose git state is unknown, even with --include-keep
- spell out that --include-keep discards uncommitted and in-progress work
2026-09-12 00:37:10 +10:00
unkin-agent 6380270ac6 Find every stale worktree, not just the managed ones
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
- discover worktrees from `git worktree list` on each source checkout, not only
  the worktree root, so hand-made ones, stale registrations and orphaned
  directories are classified too
- normalise each candidate to its main checkout, so a linked worktree in the
  source root cannot offer up the repo's real checkout
- keep locked, mid-rebase and detached-with-unique-commits worktrees, whose
  removal would destroy state nothing else holds
- name the retained branch in every unproven verdict
- add --no-fetch, --json, --include-unmanaged and --include-keep
2026-09-12 00:16:32 +10:00
benvin 6d0e954cce Merge pull request 'Add agentws prune' (#13) from benvin/agentws-prune into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #13
2026-09-10 21:50:20 +10:00
unkin-agent 387653a3c0 Distrust origin/<branch> when prune's fetch fails
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
A stale remote-tracking ref survives a failed fetch and the next successful
--prune deletes it, so it cannot prove a branch's commits survive upstream.
Record whether the pruning fetch succeeded and gate the origin/<branch>
existence and containment proofs on it; a failed fetch removes the worktree and
keeps the branch. Local-object proofs and the merged head SHA are unaffected.
2026-09-10 00:18:54 +10:00
unkin-agent c1c02c01cf Require git proof before prune deletes a branch
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
A merged or closed PR no longer authorises a delete on its own: HEAD must
be contained in the PR's head commit or in origin/<branch>, otherwise the
worktree goes and the branch stays. Branch deletion runs `git branch -d`
first and falls back to -D only for a proven branch.

Reword the cherry check to say patches reached the default branch's
history, print the verdict --keep-branches will actually perform, and warn
when a PR listing hits the pagination cap instead of reading it as "no PR".
2026-09-10 00:00:22 +10:00
unkin-agent 4bbeaae8f0 add agentws prune
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Agents leave their managed worktrees behind, and `agentws rm` takes one path at
a time with no idea whether a branch's work is safely upstream, so clearing an
accumulation by hand risks destroying unmerged commits.

- classify every managed worktree: dirty, PR open, upstream, or unproven
- remove only what is safe; delete the local branch only when work is upstream
- prove "upstream" with merge-base and git cherry, so squash merges count
- match a PR by head.label, which survives the branch deletion a merge does
- dry run by default; --yes applies, --keep-branches spares every branch
- read the Gitea path from origin's URL rather than assuming the owner
2026-09-09 23:41:09 +10:00
benvin 5c0eb1e899 Merge pull request 'Abort watchpr when a poll can no longer see the PR' (#12) from benvin/watchpr-terminal-errors into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #12
2026-09-09 23:13:17 +10:00
unkin-agent d04c5aa58d Scope watchpr's terminal 404 to the PR lookup
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Only a 404 from GetPR means the PR is gone. A 404 from any other call
can be a proxy or ingress blip, so it now warns and counts against the
consecutive-failure cap instead of killing the watch on first sight.
2026-09-09 22:55:56 +10:00
benvin 7c6ec361ae Merge pull request 'Accept a bare integer as seconds for watchpr --interval' (#11) from benvin/watchpr-interval-units into main
Reviewed-on: #11
2026-09-09 22:42:24 +10:00
unkin-agent 46dfe48adc Abort watchpr when a poll can no longer see the PR
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
- treat a mid-run 404 on a tracked PR as terminal
- cap consecutive transient poll failures at 20 per PR
- reset the failure count on a successful poll
- export IsNotFound for callers to classify the abort
2026-09-09 22:41:27 +10:00
unkin-agent 71e42811fb Accept a bare integer as seconds for watchpr --interval
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
- add agent.ParseDurationFlag: bare integer means seconds, duration strings still parse
- take --interval as a string and parse it in the command
- reject unparseable and non-positive values with an error naming the flag
2026-09-09 22:38:09 +10:00
benvin 985b58c406 Merge pull request 'Re-mint watchpr's Gitea token when it expires' (#10) from benvin/watchpr-auth-expiry into main
Reviewed-on: #10
2026-09-09 22:34:15 +10:00
unkin-agent 7ef0e28e96 Re-mint watchpr's Gitea token when it expires
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Vault-minted Gitea tokens last ~1h, far less than a watch, and every poll
past expiry 401'd into a warning while watchpr looked healthy.

- retry a rejected request once with a freshly minted token
- abort the watch when the fresh token is rejected too
- poll anonymously when no token can be minted, mint only on a real 401/403
2026-09-09 21:15:46 +10:00
20 changed files with 5157 additions and 87 deletions
+10 -3
View File
@@ -13,10 +13,11 @@ repos:
rev: v0.5.1
hooks:
- id: go-fmt
- id: go-unit-tests
# go-vet at the module level (dnephin's go-vet runs at repo root, which has no
# .go files here since both tools live under cmd/). The CI pre-commit image
# go vet and go test at the module level (dnephin's run at repo root, which has
# no .go files here since both tools live under cmd/, and its go-unit-tests
# caps every package at 30s and re-runs the whole module once per file batch —
# the git-fixture tests outgrew both). The CI pre-commit image
# (almalinux9-gobuilder) has go installed.
- repo: local
hooks:
@@ -26,3 +27,9 @@ repos:
language: system
types: [go]
pass_filenames: false
- id: go-test-mod
name: go test (module)
entry: go test ./...
language: system
types: [go]
pass_filenames: false
+56 -10
View File
@@ -8,9 +8,11 @@ from Vault, so actions are attributed to the agent rather than to whoever runs
the tool. Setting `AGENT_LOGIN` selects a different agent identity, so a service
like repospawner can run these tools as itself.
- **`agentpr`** — create pull requests and post PR comments as `unkin-agent`
(fixes the "tea posts as Ben" attribution problem). Subcommands:
`pr create`, `pr comment`, `whoami`.
- **`agentpr`** — create and edit pull requests and issues, close and reopen
issues, and post comments on either, as `unkin-agent` (fixes the "tea posts
as Ben" attribution problem). Subcommands: `pr create`, `pr comment`,
`pr edit`, `issue create`, `issue comment`, `issue edit`, `issue close`,
`issue reopen`, `whoami`.
- **`watchpr`** — poll one or more PRs and exit when a tracked PR changes
meaningfully: it merges/closes, gets a new non-agent comment, its CI fails,
or it loses mergeability. Benign transitions (CI pending→success, the agent's
@@ -19,7 +21,7 @@ like repospawner can run these tools as itself.
repos into the source root (`~/src/prodenv/<repo>`), creates worktrees under
the worktree root (`~/.cache/agentws/<repo>__<branch>`), and authenticates
clone/fetch/push via an ephemeral credential helper. Subcommands: `new`,
`list`, `rm`, `clean`, `token`, `credential`.
`list`, `rm`, `prune`, `clean`, `token`, `credential`.
All tools are separate `main` packages under `cmd/` and share the
`internal/agent` package (Vault AppRole login, Gitea REST client, PR-ref
@@ -28,14 +30,15 @@ parsing, watch-state comparison, git worktree helpers).
## Structure
```
cmd/agentpr/main.go # agentpr CLI (pr create / pr comment / whoami)
cmd/agentpr/main.go # agentpr CLI (pr + issue create/comment/edit, issue close/reopen, whoami)
cmd/watchpr/main.go # watchpr CLI (poll + meaningful-change exit)
cmd/agentws/main.go # agentws CLI (new / list / rm / clean / token / credential)
cmd/agentws/prune.go # agentws prune (classify worktrees, remove the safe ones)
cmd/agentvault/main.go # agentvault CLI (seed-outpost / seed-oauth)
internal/agent/ # shared plumbing:
token.go # env config + in-process Gitea-token cache
vault.go # AppRole login + read the gitea creds path
gitea.go # Gitea REST client (PR create/get, comments, status, whoami)
gitea.go # Gitea REST client (PR/issue create/edit/get, issue state, comments, status, whoami)
parse.go # owner/repo#N and owner/repo parsing
watch.go # PRState snapshot + MeaningfulChange comparison
git.go # git worktree/clone/fetch helpers (os/exec, no go-git)
@@ -128,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
@@ -174,12 +178,54 @@ wrapped per stage (login / read denied / write denied) via `ErrVaultDenied`.
## Gotchas
- `watchpr` exits 0 with no output changes on `--once` (just prints state).
- The token cache is process-wide (`sync.Once`); tests call the unexported
`fetchGiteaToken` to avoid it.
- 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); 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
Authentik `view_key` responses are reported without their bodies, and
`seed-oauth` reports key names only.
- `--rotate` regenerates the `client_secret` too, which then no longer matches
the IdP provider unless that is rotated alongside.
- `agentws prune` is a dry run unless `--yes`. It matches a branch to its PR on
`head.label`: Gitea rewrites `head.ref` to `refs/pull/<n>/head` once the branch
is deleted, which merging does, so `head.ref` matching misses every merged PR.
Git signals (`merge-base --is-ancestor`, `git cherry`) are authoritative and
offline-safe; an unreachable Gitea only means no branch gets deleted without
git proof. A PR's state never authorises a branch delete on its own — HEAD
must be contained in the PR's head commit or in `origin/<branch>`, otherwise
the worktree goes and the branch stays. `origin/<branch>` is only evidence when
this run's pruning fetch succeeded; a failed fetch leaves stale tracking refs,
so those verdicts fall back to keeping the branch.
- `agentws prune` discovers worktrees from the worktree root *and* from
`git worktree list` on each source checkout, merging the two so git's own
`locked`/`prunable` flags reach entries the directory scan already found.
Removing a worktree is only safe because the local branch keeps its commits, so
the cases with no branch to fall back on are kept: a detached HEAD carrying
commits on no remote, a locked checkout, or one with a sequencer operation
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.
- 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.
+146 -3
View File
@@ -6,7 +6,8 @@ token from Vault, so automated PRs, comments and pushes are attributed to the
agent — not to whoever happens to run the command. Set `AGENT_LOGIN` to act as a
different agent identity.
- **`agentpr`** — create pull requests and post PR comments as the agent user.
- **`agentpr`** — create and edit pull requests and issues, close and reopen
issues, and post comments on either, as the agent user.
- **`watchpr`** — poll one or more PRs and exit when one changes in a way worth
acting on.
- **`agentws`** — manage per-branch git worktrees for `unkin-agent`, cloning
@@ -50,6 +51,29 @@ agentpr pr create --repo unkin/argocd-apps \
# Comment on a PR
agentpr pr comment --repo unkin/argocd-apps --pr 42 --body "Rebased, CI green."
# Edit a PR's title and/or body; an omitted flag is left unchanged
agentpr pr edit --repo unkin/argocd-apps --pr 42 --body "Adds the ServiceAccount ..."
agentpr pr edit --repo unkin/argocd-apps --pr 42 --title "Add woodpecker SA"
# prints: #<number> <html_url>
# File an issue (--body optional)
agentpr issue create --repo unkin/argocd-apps \
--title "Woodpecker SA missing" --body "The pipeline fails with ..."
# prints: #<number> <html_url>
# Comment on an issue (the same Gitea endpoint `pr comment` posts to)
agentpr issue comment --repo unkin/argocd-apps --issue 43 --body "Fixed in #44."
# Edit an issue's title and/or body; an omitted flag is left unchanged
agentpr issue edit --repo unkin/argocd-apps --issue 43 --body "The pipeline fails with ..."
# prints: #<number> <html_url>
# Close or reopen an issue; an issue already in that state is an error, not a
# silent success
agentpr issue close --repo unkin/argocd-apps --issue 43
agentpr issue reopen --repo unkin/argocd-apps --issue 43
# prints: #<number> <state> <html_url>
agentpr --version
agentpr --help
```
@@ -70,6 +94,9 @@ watchpr unkin/argocd-apps#42
# Multiple PRs, custom interval; refs accept #N or :N
watchpr --interval 30s unkin/argocd-apps#42 unkin/terraform-vault:98
# --interval takes a duration (30s, 2m, 1h30m) or a bare number of seconds
watchpr --interval 30 unkin/argocd-apps#42
# One-shot: print current state and exit 0 (great for scripts)
watchpr --once unkin/argocd-apps#42
watchpr --once --json unkin/argocd-apps#42
@@ -78,6 +105,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
@@ -88,10 +134,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)
@@ -101,6 +147,13 @@ agentws list
agentws rm benvin/my-change
agentws rm ~/.cache/agentws/argocd-apps__benvin-my-change --delete-branch
# Classify every worktree found; dry run unless --yes is given
agentws prune
agentws prune --json
agentws prune --no-fetch
agentws prune --yes
agentws prune --yes --keep-branches
# Remove every managed worktree and prune each source repo
agentws clean
@@ -108,6 +161,96 @@ 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
directories under the worktree root, and `git worktree list` on every source
checkout — so hand-made worktrees, stale registrations whose directory is gone,
and leftover directories whose source repo was deleted all show up too.
It then decides, per worktree, whether its work is safely upstream:
| Signal (first match wins) | Verdict |
|---|---|
| working tree gone (registration only) | prune the registration |
| backing repo gone | delete the leftover directory |
| locked by `git worktree lock` | keep |
| rebase, merge, cherry-pick, revert or bisect in progress | keep |
| uncommitted, staged or untracked changes | keep |
| branch has an open PR | keep |
| tip contained in `origin/<default>` | remove worktree + local branch |
| every commit patch-equivalent to one in `origin/<default>`'s history | remove worktree + local branch |
| PR merged **and** HEAD contained in the PR's head commit (or in a verified `origin/<branch>`) | remove worktree + local branch |
| PR closed **and** HEAD contained in a verified `origin/<branch>` | remove worktree + local branch |
| detached HEAD carrying commits on no remote | keep |
| anything else | remove worktree, keep the branch |
A branch is deleted only where git proves its commits survive elsewhere. PR
state alone never authorises that: a merged or closed PR whose branch picked up
commits since keeps its branch, because those commits exist nowhere but here.
The delete runs `git branch -d` first so git's own unmerged check is a backstop,
falling back to `-D` only for a proven branch — squash merges keep the guard
tripping even once the work has landed.
Patch equivalence comes from `git cherry`, which these squash-merging repos need
because a merged branch's commits carry different SHAs upstream. It proves the
patches reached the default branch's history at some point — a later revert
still counts — not that they stand at its tip.
`origin/<branch>` counts as evidence only when this run's `git fetch --prune`
succeeded. A tracking ref left over from an earlier fetch may name a branch that
is already gone upstream and is itself due for deletion, so a failed fetch
downgrades those verdicts to `remove` and keeps the branch. Proofs that read
only local objects — containment in `origin/<default>`, patch equivalence, and
containment in a merged PR's head SHA — stand on their own.
Gitea PR state only adds to the git answer: when it cannot be reached, prune
says so and never deletes a branch it could not prove, and a PR listing that
hits the pagination cap is reported rather than read as "no PR". Matching a
branch to its PR uses `head.label`, since Gitea rewrites `head.ref` to
`refs/pull/<n>/head` once the branch is deleted on merge.
The last row is safe only because the local branch keeps the commits, so the
reason names the branch it is relying on. A detached HEAD has no such branch, so
unique commits there are kept instead.
Output is a table (`REPO BRANCH PATH VERDICT REASON`) with every verdict's reason
spelled out, or `--json` for scripting. Neither form needs a terminal.
| Flag | Effect |
|---|---|
| `--yes` | apply the plan; without it nothing is touched |
| `--keep-branches` | remove worktrees only; verdicts print as `remove` |
| `--no-fetch` | judge against the refs already on disk, for offline use |
| `--json` | emit the report as JSON on stdout, notes on stderr |
| `--include-unmanaged` | also remove worktrees outside the worktree root |
| `--include-keep` | dangerous: also remove worktrees classified `keep`, destroying uncommitted and in-progress work |
Without `--include-unmanaged` a hand-made worktree is reported and then skipped,
naming the flag that would remove it. `--include-keep` is the only way past a
`keep`. It leaves the branch, so committed work outlives the worktree, but
`git worktree remove --force` discards a dirty working tree and a paused
rebase's sequencer state without a word, and no branch was carrying those.
A directory under the worktree root is deleted outright only when its git dir
and the repo's shared `.git` are both proven absent by `stat`. When git merely
fails to answer for a checkout, the verdict is `keep` with the error as its
reason: an unread state is never a dead one.
### Auth / credential-helper design
Gitea tokens minted from Vault are short-lived (~1h), so `agentws` never
+218 -14
View File
@@ -5,6 +5,12 @@
//
// agentpr pr create --repo owner/repo --base main --head feature --title T --body B
// agentpr pr comment --repo owner/repo --pr 12 --body "..."
// agentpr pr edit --repo owner/repo --pr 12 --title T --body B
// agentpr issue create --repo owner/repo --title T --body B
// agentpr issue comment --repo owner/repo --issue 12 --body "..."
// agentpr issue edit --repo owner/repo --issue 12 --title T --body B
// agentpr issue close --repo owner/repo --issue 12
// agentpr issue reopen --repo owner/repo --issue 12
// agentpr whoami
package main
@@ -33,14 +39,14 @@ func main() {
func newRootCmd() *cobra.Command {
root := &cobra.Command{
Use: "agentpr",
Short: "Manage Gitea PRs and comments as an agent user.",
Long: "agentpr manages Gitea pull requests and comments as an agent user, using a\nGitea token minted from Vault (AppRole login + gitea/creds/<AGENT_LOGIN>).\nSet AGENT_LOGIN to act as another agent identity, or GITEA_CREDS_PATH to name\nthe Vault creds path outright.",
Short: "Manage Gitea PRs, issues and comments as an agent user.",
Long: "agentpr manages Gitea pull requests, issues and comments as an agent user,\nusing a Gitea token minted from Vault (AppRole login + gitea/creds/<AGENT_LOGIN>).\nSet AGENT_LOGIN to act as another agent identity, or GITEA_CREDS_PATH to name\nthe Vault creds path outright.",
Version: version,
SilenceUsage: true,
}
root.SetVersionTemplate("{{.Version}}\n")
root.AddCommand(newPRCmd(), newWhoamiCmd(), newVersionCmd())
root.AddCommand(newPRCmd(), newIssueCmd(), newWhoamiCmd(), newVersionCmd())
return root
}
@@ -56,9 +62,24 @@ func client() (*agent.GiteaClient, error) {
func newPRCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "pr",
Short: "Create PRs and post PR comments",
Short: "Create and edit PRs, and post PR comments",
}
cmd.AddCommand(newPRCreateCmd(), newPRCommentCmd())
cmd.AddCommand(newPRCreateCmd(), newCommentCmd("pr", "PR", "Post a comment on a pull request"), newPREditCmd())
return cmd
}
func newIssueCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "issue",
Short: "File, edit, close and reopen issues, and post issue comments",
}
cmd.AddCommand(
newIssueCreateCmd(),
newCommentCmd("issue", "issue", "Post a comment on an issue"),
newIssueEditCmd(),
newIssueStateCmd("close", "Close an issue", agent.IssueStateClosed),
newIssueStateCmd("reopen", "Reopen a closed issue", agent.IssueStateOpen),
)
return cmd
}
@@ -103,12 +124,56 @@ func newPRCreateCmd() *cobra.Command {
return cmd
}
func newPRCommentCmd() *cobra.Command {
// newCommentCmd builds a comment command whose number flag is named numFlag.
// Gitea backs every PR with an issue of the same number and serves comments
// from the issue endpoint, so `pr comment` and `issue comment` are one command
// under two flag names rather than two implementations that could drift.
func newCommentCmd(numFlag, noun, short string) *cobra.Command {
var repo, body string
var pr int
var number int
cmd := &cobra.Command{
Use: "comment",
Short: "Post a comment on a pull request",
Short: short,
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if number <= 0 {
return fmt.Errorf("--%s must be a positive %s number", numFlag, noun)
}
if body == "" {
return fmt.Errorf("--body is required")
}
c, err := client()
if err != nil {
return err
}
cm, err := c.CreateComment(owner+"/"+name, number, body)
if err != nil {
return err
}
fmt.Printf("comment %d posted on %s/%s#%d\n", cm.ID, owner, name, number)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&number, numFlag, 0, noun+" number (required)")
f.StringVar(&body, "body", "", "Comment body (required)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired(numFlag)
_ = cmd.MarkFlagRequired("body")
return cmd
}
func newPREditCmd() *cobra.Command {
var repo, title, body string
var pr int
cmd := &cobra.Command{
Use: "edit",
Short: "Edit a pull request's title and/or body",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
@@ -118,28 +183,167 @@ func newPRCommentCmd() *cobra.Command {
if pr <= 0 {
return fmt.Errorf("--pr must be a positive PR number")
}
if body == "" {
return fmt.Errorf("--body is required")
opts, err := editOptions(cmd, title, body)
if err != nil {
return err
}
c, err := client()
if err != nil {
return err
}
cm, err := c.CreateComment(owner+"/"+name, pr, body)
updated, err := c.EditPR(owner+"/"+name, pr, opts)
if err != nil {
return err
}
fmt.Printf("comment %d posted on %s/%s#%d\n", cm.ID, owner, name, pr)
fmt.Printf("#%d %s\n", updated.Number, updated.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&pr, "pr", 0, "PR number (required)")
f.StringVar(&body, "body", "", "Comment body (required)")
f.StringVar(&title, "title", "", "New PR title (unchanged when omitted)")
f.StringVar(&body, "body", "", "New PR body (unchanged when omitted)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("pr")
_ = cmd.MarkFlagRequired("body")
return cmd
}
// editOptions turns the --title/--body flags actually given into an edit
// payload. Only the flags present are sent: omitting --title must leave the
// title as it is, not blank it.
func editOptions(cmd *cobra.Command, title, body string) (agent.EditOptions, error) {
var opts agent.EditOptions
if cmd.Flags().Changed("title") {
// Gitea ignores an empty title, so sending one would report success
// while changing nothing.
if title == "" {
return opts, fmt.Errorf("--title cannot be empty: a title can be set but not cleared")
}
opts.Title = &title
}
if cmd.Flags().Changed("body") {
opts.Body = &body
}
if opts.Title == nil && opts.Body == nil {
return opts, fmt.Errorf("at least one of --title or --body is required")
}
return opts, nil
}
func newIssueCreateCmd() *cobra.Command {
var repo, title, body string
cmd := &cobra.Command{
Use: "create",
Short: "File an issue",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if title == "" {
return fmt.Errorf("--title is required")
}
c, err := client()
if err != nil {
return err
}
issue, err := c.CreateIssue(owner+"/"+name, agent.CreateIssueOptions{
Title: title,
Body: body,
})
if err != nil {
return err
}
fmt.Printf("#%d %s\n", issue.Number, issue.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.StringVar(&title, "title", "", "Issue title (required)")
f.StringVar(&body, "body", "", "Issue body")
_ = cmd.MarkFlagRequired("repo")
return cmd
}
func newIssueEditCmd() *cobra.Command {
var repo, title, body string
var issue int
cmd := &cobra.Command{
Use: "edit",
Short: "Edit an issue's title and/or body",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if issue <= 0 {
return fmt.Errorf("--issue must be a positive issue number")
}
opts, err := editOptions(cmd, title, body)
if err != nil {
return err
}
c, err := client()
if err != nil {
return err
}
updated, err := c.EditIssue(owner+"/"+name, issue, opts)
if err != nil {
return err
}
fmt.Printf("#%d %s\n", updated.Number, updated.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&issue, "issue", 0, "Issue number (required)")
f.StringVar(&title, "title", "", "New issue title (unchanged when omitted)")
f.StringVar(&body, "body", "", "New issue body (unchanged when omitted)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("issue")
return cmd
}
// newIssueStateCmd builds `issue close` and `issue reopen`, which differ only
// in the state they ask for. An issue already in that state is an error, not a
// silent success: Gitea answers the PATCH with 200 either way.
func newIssueStateCmd(use, short, state string) *cobra.Command {
var repo string
var issue int
cmd := &cobra.Command{
Use: use,
Short: short,
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if issue <= 0 {
return fmt.Errorf("--issue must be a positive issue number")
}
c, err := client()
if err != nil {
return err
}
updated, err := c.SetIssueState(owner+"/"+name, issue, state)
if err != nil {
return err
}
fmt.Printf("#%d %s %s\n", updated.Number, updated.State, updated.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&issue, "issue", 0, "Issue number (required)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("issue")
return cmd
}
+186
View File
@@ -2,7 +2,10 @@ package main
import (
"io"
"strings"
"testing"
"github.com/spf13/cobra"
)
// A malformed --repo must fail the command (so main exits non-zero). ParseRepo
@@ -16,3 +19,186 @@ func TestExecuteBadRepoErrors(t *testing.T) {
t.Fatal("Execute() = nil, want error for a malformed --repo")
}
}
// `pr edit` with neither --title nor --body has nothing to send; it must fail
// with a usage error before any Vault/Gitea call, so this stays hermetic.
func TestPREditRequiresTitleOrBody(t *testing.T) {
cmd := newRootCmd()
cmd.SetArgs([]string{"pr", "edit", "--repo", "unkin/repo", "--pr", "7"})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if err == nil {
t.Fatal("Execute() = nil, want an error when neither --title nor --body is given")
}
if !strings.Contains(err.Error(), "--title or --body") {
t.Errorf("error = %q, want it to name the missing flags", err)
}
}
// Gitea silently ignores an empty title, so `pr edit --title ""` would report
// success while changing nothing; it must fail before any Vault/Gitea call.
func TestPREditRejectsEmptyTitle(t *testing.T) {
cmd := newRootCmd()
cmd.SetArgs([]string{"pr", "edit", "--repo", "unkin/repo", "--pr", "7", "--title", ""})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if err == nil {
t.Fatal("Execute() = nil, want an error for an empty --title")
}
if !strings.Contains(err.Error(), "--title cannot be empty") {
t.Errorf("error = %q, want it to reject the empty title", err)
}
}
// execute runs the command tree with args, discarding output, so tests assert
// on the error alone. Every case here fails before any Vault/Gitea call.
func execute(args ...string) error {
cmd := newRootCmd()
cmd.SetArgs(args)
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
return cmd.Execute()
}
// An issue needs a title; Gitea rejects an empty one, so the command must too.
func TestIssueCreateRequiresTitle(t *testing.T) {
err := execute("issue", "create", "--repo", "unkin/repo", "--body", "b")
if err == nil {
t.Fatal("Execute() = nil, want an error when --title is missing")
}
if !strings.Contains(err.Error(), "--title is required") {
t.Errorf("error = %q, want it to name the missing flag", err)
}
}
// --repo is required, and cobra must reject its absence before anything reaches
// for a token.
func TestIssueCreateRequiresRepo(t *testing.T) {
err := execute("issue", "create", "--title", "t")
if err == nil {
t.Fatal("Execute() = nil, want an error when --repo is missing")
}
if !strings.Contains(err.Error(), "repo") {
t.Errorf("error = %q, want it to name the missing flag", err)
}
}
func TestIssueCreateBadRepoErrors(t *testing.T) {
if err := execute("issue", "create", "--repo", "not-a-repo", "--title", "t"); err == nil {
t.Fatal("Execute() = nil, want error for a malformed --repo")
}
}
// `issue comment` addresses the issue by --issue, not --pr, and needs it.
func TestIssueCommentRequiresIssueNumber(t *testing.T) {
err := execute("issue", "comment", "--repo", "unkin/repo", "--body", "hi")
if err == nil {
t.Fatal("Execute() = nil, want an error when --issue is missing")
}
if !strings.Contains(err.Error(), "issue") {
t.Errorf("error = %q, want it to name the missing --issue flag", err)
}
}
func TestIssueEditRequiresTitleOrBody(t *testing.T) {
err := execute("issue", "edit", "--repo", "unkin/repo", "--issue", "12")
if err == nil {
t.Fatal("Execute() = nil, want an error when neither --title nor --body is given")
}
if !strings.Contains(err.Error(), "--title or --body") {
t.Errorf("error = %q, want it to name the missing flags", err)
}
}
func TestIssueEditRejectsEmptyTitle(t *testing.T) {
err := execute("issue", "edit", "--repo", "unkin/repo", "--issue", "12", "--title", "")
if err == nil {
t.Fatal("Execute() = nil, want an error for an empty --title")
}
if !strings.Contains(err.Error(), "--title cannot be empty") {
t.Errorf("error = %q, want it to reject the empty title", err)
}
}
// PRs and issues share Gitea's comment endpoint, so both comment commands are
// built from one constructor: they must stay identical apart from the flag
// naming the number.
func TestCommentCommandsStayInStep(t *testing.T) {
find := func(group string) *cobra.Command {
t.Helper()
cmd, _, err := newRootCmd().Find([]string{group, "comment"})
if err != nil || cmd.Name() != "comment" {
t.Fatalf("%s comment not found: %v", group, err)
}
return cmd
}
has := func(cmd *cobra.Command, name string) bool { return cmd.Flags().Lookup(name) != nil }
prCmd, issueCmd := find("pr"), find("issue")
for _, name := range []string{"repo", "body"} {
if !has(prCmd, name) || !has(issueCmd, name) {
t.Errorf("--%s missing: pr=%t issue=%t", name, has(prCmd, name), has(issueCmd, name))
}
}
if !has(prCmd, "pr") || has(prCmd, "issue") {
t.Error("pr comment must take --pr and only --pr")
}
if !has(issueCmd, "issue") || has(issueCmd, "pr") {
t.Error("issue comment must take --issue and only --issue")
}
}
// Both state commands need a repo and an issue number; cobra and the RunE
// guard must reject their absence before anything reaches for a token.
func TestIssueStateCommandsRequireFlags(t *testing.T) {
for _, verb := range []string{"close", "reopen"} {
t.Run(verb+" without --issue", func(t *testing.T) {
err := execute("issue", verb, "--repo", "unkin/repo")
if err == nil {
t.Fatal("Execute() = nil, want an error when --issue is missing")
}
if !strings.Contains(err.Error(), "issue") {
t.Errorf("error = %q, want it to name the missing --issue flag", err)
}
})
t.Run(verb+" without --repo", func(t *testing.T) {
err := execute("issue", verb, "--issue", "12")
if err == nil {
t.Fatal("Execute() = nil, want an error when --repo is missing")
}
if !strings.Contains(err.Error(), "repo") {
t.Errorf("error = %q, want it to name the missing --repo flag", err)
}
})
t.Run(verb+" with a zero --issue", func(t *testing.T) {
err := execute("issue", verb, "--repo", "unkin/repo", "--issue", "0")
if err == nil {
t.Fatal("Execute() = nil, want an error for a non-positive issue number")
}
if !strings.Contains(err.Error(), "--issue must be a positive") {
t.Errorf("error = %q, want it to reject the issue number", err)
}
})
t.Run(verb+" with a malformed --repo", func(t *testing.T) {
if err := execute("issue", verb, "--repo", "not-a-repo", "--issue", "12"); err == nil {
t.Fatal("Execute() = nil, want error for a malformed --repo")
}
})
}
}
// Closing and reopening are issue-only: a PR is closed by a human, so the pr
// group must not grow these verbs by accident.
func TestPRHasNoStateCommands(t *testing.T) {
for _, verb := range []string{"close", "reopen"} {
if cmd, _, err := newRootCmd().Find([]string{"pr", verb}); err == nil && cmd.Name() == verb {
t.Errorf("pr %s exists; closing a PR is not agentpr's to do", verb)
}
cmd, _, err := newRootCmd().Find([]string{"issue", verb})
if err != nil || cmd.Name() != verb {
t.Fatalf("issue %s not found: %v", verb, err)
}
}
}
+375 -23
View File
@@ -13,6 +13,8 @@
// agentws new <repo> [--branch benvin/<name>] [--from <base-branch>]
// agentws list
// agentws rm <path-or-branch> [--delete-branch]
// agentws prune [--yes] [--keep-branches] [--no-fetch] [--json]
// [--include-unmanaged] [--include-keep]
// agentws clean
// agentws token
// agentws credential get # git credential-helper protocol on stdin
@@ -24,6 +26,7 @@ import (
"io"
"os"
"path/filepath"
"sort"
"strings"
"git.unkin.net/unkin/agent-tools/internal/agent"
@@ -54,6 +57,7 @@ func newRootCmd() *cobra.Command {
newNewCmd(),
newListCmd(),
newRmCmd(),
newPruneCmd(),
newCleanCmd(),
newTokenCmd(),
newCredentialCmd(),
@@ -157,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
@@ -172,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
}
@@ -195,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
},
}
@@ -208,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 {
@@ -226,7 +292,14 @@ func newListCmd() *cobra.Command {
return nil
}
for _, w := range managed {
_, _ = fmt.Fprintf(out, "%s\t%s\t%s\n", w.repo, w.branch, w.path)
branch := w.branch
switch {
case w.orphan:
branch = "(orphan)"
case w.inspectErr != nil:
branch = "(unreadable)"
}
_, _ = fmt.Fprintf(out, "%s\t%s\t%s\n", w.repo, branch, w.path)
}
return nil
},
@@ -239,10 +312,31 @@ type managedWt struct {
branch string
path string
srcDir string
// managed is false for worktrees found via `git worktree list` that live
// outside the worktree root, i.e. somebody made them by hand.
managed bool
// detached is true when the worktree has no branch to fall back on, so its
// commits die with the checkout.
detached bool
// locked records git's own "do not remove me" marker.
locked bool
// missing is a registration whose working tree is gone: nothing to inspect,
// nothing to lose.
missing bool
// orphan is a directory under the worktree root whose backing git dir is
// proven gone, so no git state can be read from it ever again.
orphan bool
// inspectErr is set when git refused to answer for a checkout and the reason
// was not a proven-absent backing repo. The state is unknown, never removable.
inspectErr error
}
// managedWorktrees scans the worktree root and resolves each entry's repo and
// branch from git so branch names are accurate (not the sanitized dir name).
// Directories whose backing repo is proven gone are returned as orphans rather
// than dropped, so callers can see (and clean up) the leftovers; a directory git
// merely failed to answer for is returned with its error instead, because an
// unread state must never be mistaken for a dead one.
func managedWorktrees() ([]managedWt, error) {
wr, err := worktreeRoot()
if err != nil {
@@ -261,24 +355,238 @@ func managedWorktrees() ([]managedWt, error) {
continue
}
path := filepath.Join(wr, e.Name())
branch, err := agent.GitCurrentBranch(path)
if err != nil {
continue // not a git worktree; skip
if _, err := os.Stat(filepath.Join(path, ".git")); err != nil {
continue // not a worktree checkout at all
}
srcDir, err := agent.SourceRepoDir(path)
if err != nil {
branch, branchErr := agent.GitCurrentBranch(path)
srcDir, srcErr := agent.SourceRepoDir(path)
if branchErr != nil || srcErr != nil {
entry := managedWt{repo: repoFromDirName(e.Name()), path: path, managed: true}
if gone, err := backingRepoGone(path); err == nil && gone {
entry.orphan = true
} else if branchErr != nil {
entry.inspectErr = branchErr
} else {
entry.inspectErr = srcErr
}
out = append(out, entry)
continue
}
out = append(out, managedWt{
repo: filepath.Base(srcDir),
branch: branch,
path: path,
srcDir: srcDir,
repo: filepath.Base(srcDir),
branch: branch,
path: path,
srcDir: srcDir,
managed: true,
detached: branch == "HEAD",
})
}
return out, nil
}
// backingRepoGone proves, by stat alone, that a worktree directory's backing
// repo no longer exists: its .git file names a git dir that is absent, and the
// repo's shared .git the git dir lived in is absent too. Only that pair licenses
// deleting the directory. Every other outcome — an unreadable .git file, a git
// dir still on disk, a stat that failed for any reason other than "not there",
// or a mere lost registration in a repo that is still present — reports false,
// so a transient or unexplained failure can never be read as "safe to delete".
func backingRepoGone(path string) (bool, error) {
dot := filepath.Join(path, ".git")
info, err := os.Lstat(dot)
if err != nil {
return false, err
}
if info.IsDir() {
return false, nil // a standalone checkout, not a linked worktree
}
data, err := os.ReadFile(dot)
if err != nil {
return false, err
}
rest, ok := strings.CutPrefix(strings.TrimSpace(string(data)), "gitdir:")
if !ok {
return false, fmt.Errorf("%s: not a worktree gitdir pointer", dot)
}
gitDir := strings.TrimSpace(rest)
if gitDir == "" {
return false, fmt.Errorf("%s: empty gitdir", dot)
}
if !filepath.IsAbs(gitDir) {
gitDir = filepath.Join(path, gitDir)
}
// The git dir is "<repo>/.git/worktrees/<name>"; both it and the shared .git
// it sits in must be absent before the repo counts as gone.
for _, dir := range []string{gitDir, filepath.Dir(filepath.Dir(gitDir))} {
if _, err := os.Stat(dir); err == nil {
return false, nil
} else if !os.IsNotExist(err) {
return false, err
}
}
return true, nil
}
// repoFromDirName recovers the repo name from the "<repo>__<branch>" layout used
// under the worktree root, for entries git can no longer answer for.
func repoFromDirName(name string) string {
if repo, _, ok := strings.Cut(name, "__"); ok {
return repo
}
return name
}
// allWorktrees is every worktree prune should consider: the managed ones under
// the worktree root, plus whatever `git worktree list` reports for the repos
// they belong to and for every checkout in the source root. The second source
// finds hand-made worktrees and stale registrations whose directory is gone, and
// carries git's own locked/prunable flags onto the entries the first source
// already found.
func allWorktrees() ([]managedWt, error) {
managed, err := managedWorktrees()
if err != nil {
return nil, err
}
wr, err := worktreeRoot()
if err != nil {
return nil, err
}
out := make([]managedWt, 0, len(managed))
index := map[string]int{}
for _, w := range managed {
index[resolvePath(w.path)] = len(out)
out = append(out, w)
}
for _, srcDir := range sourceRepos(managed) {
wts, err := agent.GitWorktreeList(srcDir)
if err != nil {
continue // not a repo any more, or unreadable; managed entries still stand
}
for _, wt := range wts {
if wt.Bare || sameDir(wt.Path, srcDir) {
continue
}
_, statErr := os.Stat(wt.Path)
entry := managedWt{
repo: filepath.Base(srcDir),
branch: worktreeBranch(wt),
path: wt.Path,
srcDir: srcDir,
managed: underRoot(wt.Path, wr),
detached: wt.Detached,
locked: wt.Locked,
missing: wt.Prunable != "" || os.IsNotExist(statErr),
}
key := resolvePath(wt.Path)
if i, ok := index[key]; ok {
// Keep the managed scan's own view, but adopt the flags only git knows.
out[i].locked = entry.locked
out[i].missing = out[i].missing || entry.missing
out[i].detached = out[i].detached || entry.detached
continue
}
index[key] = len(out)
out = append(out, entry)
}
}
sort.Slice(out, func(i, j int) bool {
if out[i].repo != out[j].repo {
return out[i].repo < out[j].repo
}
return out[i].path < out[j].path
})
return out, nil
}
// resolvePath is a path key that matches however git spells the same directory.
func resolvePath(path string) string {
if p, err := filepath.EvalSymlinks(path); err == nil {
return p
}
return filepath.Clean(path)
}
// worktreeBranch names a worktree's branch, reporting a detached checkout as
// "HEAD" so it reads the same as GitCurrentBranch does.
func worktreeBranch(wt agent.Worktree) string {
if wt.Branch != "" {
return wt.Branch
}
return "HEAD"
}
// sourceRepos is every repo to enumerate worktrees from: the ones the managed
// worktrees point back at, plus every git checkout directly under the source
// root (so a repo with only hand-made worktrees is still covered). Each is
// normalised to its main checkout, because a directory in the source root may
// itself be a linked worktree — enumerating from there would report the repo's
// real checkout as a removable worktree of itself.
func sourceRepos(managed []managedWt) []string {
seen := map[string]bool{}
var dirs []string
add := func(dir string) {
if dir == "" {
return
}
if main, err := agent.SourceRepoDir(dir); err == nil {
dir = main
}
key := resolvePath(dir)
if seen[key] {
return
}
seen[key] = true
dirs = append(dirs, dir)
}
for _, w := range managed {
add(w.srcDir)
}
if sr, err := srcRoot(); err == nil {
if entries, err := os.ReadDir(sr); err == nil {
for _, e := range entries {
if !e.IsDir() {
continue
}
dir := filepath.Join(sr, e.Name())
if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil {
add(dir)
}
}
}
}
sort.Strings(dirs)
return dirs
}
// sameDir compares two paths after resolving symlinks, because git reports
// worktree paths fully resolved while our own paths may not be.
func sameDir(a, b string) bool {
if a == b {
return true
}
ra, errA := filepath.EvalSymlinks(a)
rb, errB := filepath.EvalSymlinks(b)
return errA == nil && errB == nil && ra == rb
}
// underRoot reports whether path sits inside root, comparing resolved paths
// because git hands back worktree paths with symlinks already resolved.
func underRoot(path, root string) bool {
if r, err := filepath.EvalSymlinks(root); err == nil {
root = r
}
if p, err := filepath.EvalSymlinks(path); err == nil {
path = p
}
rel, err := filepath.Rel(root, path)
if err != nil {
return false
}
return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
}
// --- rm -------------------------------------------------------------------
func newRmCmd() *cobra.Command {
@@ -294,7 +602,8 @@ func newRmCmd() *cobra.Command {
if err != nil {
return err
}
return removeWorktree(cmd.OutOrStdout(), wt, deleteBranch)
// Naming one worktree to delete is explicit, so rm keeps the force fallback.
return removeWorktree(cmd.OutOrStdout(), wt, deleteBranch, true)
},
}
cmd.Flags().BoolVar(&deleteBranch, "delete-branch", false, "Also delete the local branch after removing the worktree")
@@ -309,20 +618,35 @@ func resolveWorktree(target string) (managedWt, error) {
}
abs, _ := filepath.Abs(target)
for _, w := range managed {
if w.path == target || w.path == abs || w.branch == target {
if w.path == target || w.path == abs || (w.branch != "" && w.branch == target) {
return w, nil
}
}
return managedWt{}, fmt.Errorf("no managed worktree matching %q (try `agentws list`)", target)
}
func removeWorktree(out io.Writer, wt managedWt, deleteBranch bool) error {
// removeWorktree removes a managed worktree and, when asked, its local branch.
// forceBranch overrides git's unmerged-branch guard, so only a caller that
// proved the commits survive elsewhere may set it.
func removeWorktree(out io.Writer, wt managedWt, deleteBranch, forceBranch bool) error {
switch {
case wt.inspectErr != nil:
// No srcDir to act through and no idea what is in there; --include-keep
// must not turn that into a delete.
return fmt.Errorf("refusing to remove %s: git state unreadable: %w", wt.path, wt.inspectErr)
case wt.orphan:
return removeOrphanDir(out, wt)
case wt.missing:
// The working tree is already gone; only the registration is left.
_, _ = fmt.Fprintf(out, "pruned stale registration %s\n", wt.path)
return agent.GitWorktreePrune(wt.srcDir)
}
if err := agent.GitWorktreeRemove(wt.srcDir, wt.path, true); err != nil {
return err
}
_, _ = fmt.Fprintf(out, "removed worktree %s\n", wt.path)
if deleteBranch {
if err := agent.GitDeleteBranch(wt.srcDir, wt.branch, true); err != nil {
if err := deleteLocalBranch(wt, forceBranch); err != nil {
return err
}
_, _ = fmt.Fprintf(out, "deleted branch %s\n", wt.branch)
@@ -334,6 +658,34 @@ func removeWorktree(out io.Writer, wt managedWt, deleteBranch bool) error {
return agent.GitWorktreePrune(wt.srcDir)
}
// removeOrphanDir deletes a worktree directory whose backing repo is gone. git
// cannot act on it, so this is a plain delete — confined to the worktree root so
// a bad path can never reach a real checkout.
func removeOrphanDir(out io.Writer, wt managedWt) error {
wr, err := worktreeRoot()
if err != nil {
return err
}
if !underRoot(wt.path, wr) || sameDir(wt.path, wr) {
return fmt.Errorf("refusing to delete %s: not inside the worktree root %s", wt.path, wr)
}
if err := os.RemoveAll(wt.path); err != nil {
return err
}
_, _ = fmt.Fprintf(out, "deleted orphaned worktree directory %s\n", wt.path)
return nil
}
// deleteLocalBranch tries the guarded delete first so git refuses to drop
// unmerged commits on its own; force is a fallback, never the first attempt.
func deleteLocalBranch(wt managedWt, force bool) error {
err := agent.GitDeleteBranch(wt.srcDir, wt.branch, false)
if err == nil || !force {
return err
}
return agent.GitDeleteBranch(wt.srcDir, wt.branch, true)
}
// --- clean ----------------------------------------------------------------
func newCleanCmd() *cobra.Command {
@@ -352,7 +704,7 @@ func newCleanCmd() *cobra.Command {
return nil
}
for _, w := range managed {
if err := removeWorktree(out, w, false); err != nil {
if err := removeWorktree(out, w, false, false); err != nil {
return err
}
}
+192
View File
@@ -0,0 +1,192 @@
package main
import (
"bytes"
"path/filepath"
"strings"
"testing"
"git.unkin.net/unkin/agent-tools/internal/agent"
)
// newFixtureOn builds the same origin/source/worktree-root layout as the prune
// fixture but with a chosen default branch, so `new` can be tested against a
// repo whose default is not "main".
func newFixtureOn(t *testing.T, defBranch string) *fixture {
t.Helper()
root := t.TempDir()
f := &fixture{
root: root,
bare: filepath.Join(root, "origin.git"),
srcDir: filepath.Join(root, "src", "repo"),
wtRoot: filepath.Join(root, "worktrees"),
}
git(t, root, "init", "--bare", "-b", defBranch, f.bare)
seed := filepath.Join(root, "seed")
git(t, root, "init", "-b", defBranch, seed)
identity(t, seed)
writeCommit(t, seed, "README.md", "hi\n", "init")
git(t, seed, "remote", "add", "origin", f.bare)
git(t, seed, "push", "-u", "origin", defBranch)
git(t, root, "clone", f.bare, f.srcDir)
identity(t, f.srcDir)
t.Setenv("AGENTWS_ROOT", f.wtRoot)
t.Setenv("AGENTWS_SRC_ROOT", filepath.Join(root, "src"))
t.Setenv("AGENTWS_OWNER", "unkin")
return f
}
// runNew invokes `agentws new repo --branch <branch>` and returns its output.
func runNew(t *testing.T, branch string, extra ...string) string {
t.Helper()
var out bytes.Buffer
cmd := newRootCmd()
cmd.SetArgs(append([]string{"new", "repo", "--branch", branch}, extra...))
cmd.SetOut(&out)
cmd.SetErr(&out)
if err := cmd.Execute(); err != nil {
t.Fatalf("new %s: %v (output %q)", branch, err, out.String())
}
return out.String()
}
// pushBranch creates branch on origin carrying one commit and returns its SHA.
func pushBranch(t *testing.T, f *fixture, branch, file, content string) string {
t.Helper()
seed := filepath.Join(f.root, "seed")
git(t, seed, "checkout", "-b", branch)
writeCommit(t, seed, file, content, "work on "+branch)
git(t, seed, "push", "origin", branch)
return git(t, seed, "rev-parse", "HEAD")
}
func wtPathFor(f *fixture, branch string) string {
return filepath.Join(f.wtRoot, agent.WorktreeDirName("repo", branch))
}
// The regression: a branch that already exists on origin must be checked out at
// origin's tip, not forked from the default branch.
func TestNewChecksOutExistingRemoteBranch(t *testing.T) {
f := newFixtureOn(t, "main")
want := pushBranch(t, f, "benvin/existing", "a.txt", "a\n")
out := runNew(t, "benvin/existing")
path := wtPathFor(f, "benvin/existing")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/existing %s", got, want)
}
if upstream := git(t, path, "rev-parse", "--abbrev-ref", "HEAD@{upstream}"); upstream != "origin/benvin/existing" {
t.Errorf("upstream = %q, want origin/benvin/existing", upstream)
}
if !contains(out, "tracking origin/benvin/existing") {
t.Errorf("output %q does not say the remote branch was checked out", out)
}
}
// A branch origin does not have is still forked from the default branch, and the
// output must say so rather than leaving the caller to guess.
func TestNewForksBranchMissingFromRemote(t *testing.T) {
f := newFixtureOn(t, "main")
want := git(t, f.srcDir, "rev-parse", "origin/main")
out := runNew(t, "benvin/fresh")
path := wtPathFor(f, "benvin/fresh")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/main %s", got, want)
}
if !contains(out, "branch benvin/fresh (new, from origin/main)") {
t.Errorf("output %q does not report a new branch", out)
}
}
// The base is the remote's own default branch, so a repo defaulting to master
// forks from master.
func TestNewForksFromMasterDefaultBranch(t *testing.T) {
f := newFixtureOn(t, "master")
want := git(t, f.srcDir, "rev-parse", "origin/master")
out := runNew(t, "benvin/on-master")
path := wtPathFor(f, "benvin/on-master")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/master %s", got, want)
}
if !contains(out, "from origin/master") {
t.Errorf("output %q does not name origin/master as the base", out)
}
}
// An existing remote branch beats --from: the flag is ignored and the caller is
// told, rather than the branch being silently re-forked.
func TestNewIgnoresFromWhenBranchIsOnRemote(t *testing.T) {
f := newFixtureOn(t, "main")
want := pushBranch(t, f, "benvin/with-from", "a.txt", "a\n")
out := runNew(t, "benvin/with-from", "--from", "main")
path := wtPathFor(f, "benvin/with-from")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/with-from %s", got, want)
}
if !contains(out, "--from main ignored") {
t.Errorf("output %q does not report the ignored --from", out)
}
}
// A local branch left behind by an earlier run must not pin the worktree to a
// commit origin has moved past.
func TestNewFastForwardsStaleLocalBranch(t *testing.T) {
f := newFixtureOn(t, "main")
stale := pushBranch(t, f, "benvin/stale", "a.txt", "a\n")
git(t, f.srcDir, "fetch", "origin")
git(t, f.srcDir, "branch", "benvin/stale", "origin/benvin/stale")
seed := filepath.Join(f.root, "seed")
writeCommit(t, seed, "b.txt", "b\n", "more work")
git(t, seed, "push", "origin", "benvin/stale")
want := git(t, seed, "rev-parse", "HEAD")
if want == stale {
t.Fatal("fixture did not move origin/benvin/stale on")
}
out := runNew(t, "benvin/stale")
path := wtPathFor(f, "benvin/stale")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/stale %s", got, want)
}
if !contains(out, "fast-forwarded stale benvin/stale") {
t.Errorf("output %q does not report the fast-forward", out)
}
}
// A local branch carrying commits origin does not have keeps them: they exist
// nowhere else, so the worktree stays put and the output says so.
func TestNewKeepsLocalCommitsAheadOfRemote(t *testing.T) {
f := newFixtureOn(t, "main")
pushBranch(t, f, "benvin/ahead", "a.txt", "a\n")
git(t, f.srcDir, "fetch", "origin")
git(t, f.srcDir, "checkout", "-b", "benvin/ahead", "origin/benvin/ahead")
writeCommit(t, f.srcDir, "local.txt", "local\n", "local only")
want := git(t, f.srcDir, "rev-parse", "HEAD")
git(t, f.srcDir, "checkout", "main")
out := runNew(t, "benvin/ahead")
path := wtPathFor(f, "benvin/ahead")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want the local tip %s", got, want)
}
if !contains(out, "1 commit not on origin/benvin/ahead") {
t.Errorf("output %q does not report the unpushed commit", out)
}
}
func contains(haystack, needle string) bool {
return strings.Contains(haystack, needle)
}
+480
View File
@@ -0,0 +1,480 @@
package main
import (
"encoding/json"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"sort"
"strings"
"text/tabwriter"
"git.unkin.net/unkin/agent-tools/internal/agent"
"github.com/spf13/cobra"
)
// Verdicts a worktree can be classified into.
const (
verdictKeep = "keep"
verdictRemove = "remove"
verdictRemoveBranch = "remove+branch"
)
// prLister is the slice of the Gitea client prune needs, so tests can drive
// classification without a live server.
type prLister interface {
ListPRs(repoPath, state string) ([]agent.PullRequest, error)
}
type pruneResult struct {
wt managedWt
verdict string
reason string
// proven records that git itself confirmed the branch's commits survive
// elsewhere; only then may a branch delete override git's own guard.
proven bool
}
// repoCtx is the per-repo state classification is decided against.
type repoCtx struct {
srcDir string
defBranch string
prs map[string]agent.PullRequest
prsKnown bool
// fetched records that this run's pruning fetch succeeded; without it an
// origin/<branch> ref may be stale and due for deletion, so it proves nothing.
fetched bool
// unfetched explains why, so a verdict can say which it was.
unfetched string
}
// pruneOpts is the knob set runPrune is driven by.
type pruneOpts struct {
apply bool
keepBranches bool
noFetch bool
jsonOut bool
includeKeep bool
includeUnmanaged bool
}
// reportEntry is the --json shape: one object per worktree, mirroring the table.
type reportEntry struct {
Repo string `json:"repo"`
Branch string `json:"branch"`
Path string `json:"path"`
Verdict string `json:"verdict"`
Reason string `json:"reason"`
Managed bool `json:"managed"`
Applied bool `json:"applied"`
}
func newPruneCmd() *cobra.Command {
var opts pruneOpts
cmd := &cobra.Command{
Use: "prune",
Short: "Classify worktrees and remove the ones whose work is safely upstream",
Long: "prune inspects every worktree it can find — the managed ones under the worktree\nroot plus whatever `git worktree list` reports for the source checkouts — and\nclassifies each against git and its Gitea pull request. It reports and changes\nnothing unless --yes is given.",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
return runPrune(cmd.OutOrStdout(), cmd.ErrOrStderr(), pruneClient(), opts)
},
}
f := cmd.Flags()
f.BoolVar(&opts.apply, "yes", false, "Actually remove worktrees (default is a dry run)")
f.BoolVar(&opts.keepBranches, "keep-branches", false, "Never delete a local branch, whatever the classification")
f.BoolVar(&opts.noFetch, "no-fetch", false, "Do not fetch; judge against the refs already on disk")
f.BoolVar(&opts.jsonOut, "json", false, "Emit JSON instead of a table")
f.BoolVar(&opts.includeKeep, "include-keep", false, "Dangerous: also remove worktrees classified keep (needs --yes). Destroys uncommitted changes and paused rebase/merge state, which no branch is carrying; only the branch itself survives")
f.BoolVar(&opts.includeUnmanaged, "include-unmanaged", false, "Also remove worktrees that live outside the worktree root")
return cmd
}
// pruneClient builds a Gitea client, falling back to anonymous access when no
// token can be minted; prune degrades to git-only signals if that fails too.
func pruneClient() prLister {
tok, err := agent.GiteaToken()
if err != nil {
tok = ""
}
return agent.NewGiteaClient(tok)
}
func runPrune(out, errOut io.Writer, prs prLister, opts pruneOpts) error {
// In JSON mode stdout carries the document alone, so notes go to stderr.
notes := out
if opts.jsonOut {
notes = errOut
}
worktrees, err := allWorktrees()
if err != nil {
return err
}
if len(worktrees) == 0 {
if opts.jsonOut {
_, _ = fmt.Fprintln(out, "[]")
return nil
}
_, _ = fmt.Fprintln(out, "no managed worktrees")
return nil
}
results := classifyAll(notes, prs, worktrees, opts)
if err := report(out, results, opts); err != nil {
return err
}
if !opts.apply {
if !opts.jsonOut {
_, _ = fmt.Fprintln(out, "dry run: nothing removed (pass --yes to apply)")
}
return nil
}
return applyPrune(notes, results, opts)
}
// classifyAll groups worktrees by source repo so each repo is fetched and its
// PRs listed once, then classifies every worktree against that repo's state.
func classifyAll(notes io.Writer, prs prLister, worktrees []managedWt, opts pruneOpts) []pruneResult {
byRepo := map[string][]managedWt{}
var results []pruneResult
for _, w := range worktrees {
switch {
case w.inspectErr != nil:
// Unknown is not gone: a checkout git refused to answer for keeps.
results = append(results, pruneResult{wt: w, verdict: verdictKeep, reason: "inspection failed: " + oneLine(w.inspectErr.Error())})
case w.orphan:
results = append(results, pruneResult{wt: w, verdict: verdictRemove, reason: "backing repo gone, no git state to read"})
default:
byRepo[w.srcDir] = append(byRepo[w.srcDir], w)
}
}
srcDirs := make([]string, 0, len(byRepo))
for dir := range byRepo {
srcDirs = append(srcDirs, dir)
}
sort.Strings(srcDirs)
for _, srcDir := range srcDirs {
ctx, err := newRepoCtx(notes, prs, srcDir, opts.noFetch)
if err != nil {
for _, w := range byRepo[srcDir] {
results = append(results, pruneResult{wt: w, verdict: verdictKeep, reason: "repo state unknown: " + oneLine(err.Error())})
}
continue
}
for _, w := range byRepo[srcDir] {
res, err := classify(w, ctx)
if err != nil {
res = pruneResult{wt: w, verdict: verdictKeep, reason: "inspection failed: " + oneLine(err.Error())}
}
results = append(results, res)
}
}
sort.SliceStable(results, func(i, j int) bool {
if results[i].wt.repo != results[j].wt.repo {
return results[i].wt.repo < results[j].wt.repo
}
return results[i].wt.path < results[j].wt.path
})
return results
}
// report writes the classification as a table or as JSON.
func report(out io.Writer, results []pruneResult, opts pruneOpts) error {
if opts.jsonOut {
entries := make([]reportEntry, 0, len(results))
for _, r := range results {
entries = append(entries, reportEntry{
Repo: r.wt.repo,
Branch: r.wt.branch,
Path: r.wt.path,
Verdict: plannedVerdict(r, opts),
Reason: r.reason,
Managed: r.wt.managed,
Applied: opts.apply && willRemove(r, opts),
})
}
enc := json.NewEncoder(out)
enc.SetIndent("", " ")
return enc.Encode(entries)
}
// Padded with spaces only, so the table reads the same with or without a TTY.
tw := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0)
_, _ = fmt.Fprintln(tw, "REPO\tBRANCH\tPATH\tVERDICT\tREASON")
for _, r := range results {
_, _ = fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\n",
dash(r.wt.repo), dash(r.wt.branch), abbrevHome(r.wt.path), plannedVerdict(r, opts), r.reason)
}
return tw.Flush()
}
// oneLine flattens a git error onto a single line so one row stays one row.
func oneLine(s string) string {
return strings.Join(strings.Fields(s), " ")
}
func dash(s string) string {
if s == "" {
return "-"
}
return s
}
// abbrevHome shortens $HOME to ~ so paths do not dominate the table.
func abbrevHome(path string) string {
home, err := os.UserHomeDir()
if err != nil || home == "" || !strings.HasPrefix(path, home+string(filepath.Separator)) {
return path
}
return "~" + path[len(home):]
}
// applyPrune performs the removals the classification authorised, reporting each
// one and collecting failures so one bad worktree does not stop the rest.
func applyPrune(out io.Writer, results []pruneResult, opts pruneOpts) error {
var errs []error
for _, r := range results {
if !willRemove(r, opts) {
if !r.wt.managed && r.verdict != verdictKeep {
_, _ = fmt.Fprintf(out, "skipped %s: outside the worktree root (pass --include-unmanaged)\n", r.wt.path)
}
continue
}
if r.verdict == verdictKeep {
_, _ = fmt.Fprintf(out, "warn: removing %s despite %q (--include-keep)\n", r.wt.path, r.reason)
}
deleteBranch := plannedVerdict(r, opts) == verdictRemoveBranch
if err := removeWorktree(out, r.wt, deleteBranch, r.proven); err != nil {
errs = append(errs, fmt.Errorf("%s: %w", r.wt.path, err))
}
}
return errors.Join(errs...)
}
// willRemove is the single gate on destruction: a keep verdict needs
// --include-keep, and a worktree outside the worktree root needs
// --include-unmanaged.
func willRemove(r pruneResult, opts pruneOpts) bool {
if !r.wt.managed && !opts.includeUnmanaged {
return false
}
if r.verdict == verdictKeep {
return opts.includeKeep
}
return true
}
// plannedVerdict is what will actually happen, so no flag prints an action it
// will not perform. A detached HEAD has no branch to delete, and a keep forced
// through with --include-keep never takes its branch with it.
func plannedVerdict(r pruneResult, opts pruneOpts) string {
if r.verdict != verdictRemoveBranch {
return r.verdict
}
if opts.keepBranches || r.wt.detached {
return verdictRemove
}
return verdictRemoveBranch
}
// newRepoCtx refreshes a source repo and collects the signals prune classifies
// against. A failed or skipped fetch and an unreachable Gitea are reported and
// tolerated: the signals that hold offline still work, and the rest are recorded
// as unverified.
func newRepoCtx(out io.Writer, prs prLister, srcDir string, noFetch bool) (repoCtx, error) {
ctx := repoCtx{srcDir: srcDir, prs: map[string]agent.PullRequest{}}
repo := filepath.Base(srcDir)
if noFetch {
ctx.unfetched = "fetch skipped"
_, _ = fmt.Fprintf(out, "warn: fetch %s skipped (--no-fetch, remote state unverified)\n", repo)
} else if err := agent.GitFetchPrune(srcDir, "origin", credentialHelperArgs()...); err != nil {
ctx.unfetched = "fetch failed"
_, _ = fmt.Fprintf(out, "warn: fetch %s: %v (remote state unverified)\n", repo, err)
} else {
ctx.fetched = true
}
def, err := agent.GitRemoteDefaultBranch(srcDir, "origin")
if err != nil {
return repoCtx{}, err
}
ctx.defBranch = def
if prs == nil {
return ctx, nil
}
list, err := prs.ListPRs(repoPath(srcDir, repo), "all")
switch {
case errors.Is(err, agent.ErrPRListTruncated):
// A branch missing from a partial listing must not read as "no PR".
_, _ = fmt.Fprintf(out, "warn: list PRs for %s: %v (older PRs unseen)\n", repo, err)
ctx.prs = prsByBranch(list)
case err != nil:
_, _ = fmt.Fprintf(out, "warn: list PRs for %s: %v (git signals only)\n", repo, err)
default:
ctx.prs = prsByBranch(list)
ctx.prsKnown = true
}
return ctx, nil
}
// repoPath is the Gitea "owner/repo" for a checkout, read from origin's URL
// because not every managed repo lives under AGENTWS_OWNER.
func repoPath(srcDir, repo string) string {
url, err := agent.GitRemoteURL(srcDir, "origin")
if err == nil && agent.RemoteHost(url) == giteaHost() {
if path, err := agent.RepoPathFromRemoteURL(url); err == nil {
return path
}
}
return owner() + "/" + repo
}
// prsByBranch indexes PRs by head branch, preferring an open PR and otherwise
// the most recent one when a branch has been used more than once.
func prsByBranch(list []agent.PullRequest) map[string]agent.PullRequest {
out := map[string]agent.PullRequest{}
for _, pr := range list {
branch := agent.PRHeadBranch(pr)
if branch == "" {
continue
}
if cur, ok := out[branch]; ok && !supersedes(pr, cur) {
continue
}
out[branch] = pr
}
return out
}
func supersedes(a, b agent.PullRequest) bool {
if a.IsOpen() != b.IsOpen() {
return a.IsOpen()
}
if a.Merged != b.Merged {
return a.Merged
}
return a.Number > b.Number
}
// headContainedIn reports whether the worktree's HEAD is reachable from ref. A
// ref that cannot be resolved proves nothing, so it reads as not contained.
func headContainedIn(dir, ref string) bool {
if ref == "" {
return false
}
ok, err := agent.GitIsAncestor(dir, "HEAD", ref)
return err == nil && ok
}
// classify applies the prune precedence. Removal must never destroy state that
// exists nowhere else: a vanished working tree is the one case with nothing to
// lose, a locked or mid-rebase checkout holds sequencer state git itself refuses
// to discard, and a dirty checkout or a detached HEAD with unique commits holds
// work no branch is carrying. Past those guards, provably-upstream work loses its
// branch too, and anything unproven keeps its branch so no commit becomes
// unreachable. A PR's state alone never authorises deleting a branch — git must
// confirm HEAD is contained in what merged or in what origin still holds, and
// origin's refs only count when this run's pruning fetch refreshed them.
func classify(wt managedWt, ctx repoCtx) (pruneResult, error) {
res := pruneResult{wt: wt}
if wt.missing {
res.verdict, res.reason = verdictRemove, "working tree gone, stale registration only"
return res, nil
}
if wt.locked {
res.verdict, res.reason = verdictKeep, "locked"
return res, nil
}
op, err := agent.GitInProgressOp(wt.path)
if err != nil {
return res, err
}
if op != "" {
res.verdict, res.reason = verdictKeep, op+" in progress"
return res, nil
}
dirty, err := agent.GitIsDirty(wt.path)
if err != nil {
return res, err
}
if dirty {
res.verdict, res.reason = verdictKeep, "dirty"
return res, nil
}
pr, hasPR := ctx.prs[wt.branch]
if hasPR && pr.IsOpen() {
res.verdict, res.reason = verdictKeep, fmt.Sprintf("PR open #%d", pr.Number)
return res, nil
}
upstream := "origin/" + ctx.defBranch
contained, err := agent.GitIsAncestor(wt.path, "HEAD", upstream)
if err != nil {
return res, err
}
if contained {
res.verdict, res.reason, res.proven = verdictRemoveBranch, "contained in "+upstream, true
return res, nil
}
unmerged, err := agent.GitUnmergedCommits(wt.path, upstream, "HEAD")
if err != nil {
return res, err
}
if unmerged == 0 {
// git cherry proves the patches reached that history, not that they stand at its tip.
res.verdict, res.reason, res.proven = verdictRemoveBranch, "patch-equivalent commits in "+upstream+" history", true
return res, nil
}
local, err := agent.GitCommitsNotOnRemotes(wt.path)
if err != nil {
return res, err
}
if wt.detached && local > 0 {
res.verdict, res.reason = verdictKeep, fmt.Sprintf("detached HEAD carrying %s on no remote", commitCount(local))
return res, nil
}
remote := "origin/" + wt.branch
onOrigin := hasPR && ctx.fetched && agent.GitRemoteBranchExists(ctx.srcDir, "origin", wt.branch)
switch {
case hasPR && pr.Merged && headContainedIn(wt.path, pr.Head.Sha):
res.verdict, res.reason, res.proven = verdictRemoveBranch, fmt.Sprintf("PR merged #%d, HEAD contained in the merged head", pr.Number), true
case hasPR && pr.Merged && onOrigin && headContainedIn(wt.path, remote):
res.verdict, res.reason, res.proven = verdictRemoveBranch, fmt.Sprintf("PR merged #%d, HEAD contained in %s", pr.Number, remote), true
case hasPR && pr.Merged && !ctx.fetched:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR merged #%d, %s so %s is unverified", pr.Number, ctx.unfetched, remote)
case hasPR && pr.Merged:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR merged #%d, local commits not in the merged head", pr.Number)
case hasPR && onOrigin && headContainedIn(wt.path, remote):
res.verdict, res.reason, res.proven = verdictRemoveBranch, fmt.Sprintf("PR closed #%d, HEAD contained in %s", pr.Number, remote), true
case hasPR && onOrigin:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR closed #%d, local commits not on %s", pr.Number, remote)
case hasPR && !ctx.fetched:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR closed #%d, %s so %s is unverified", pr.Number, ctx.unfetched, remote)
case hasPR:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR closed #%d, branch gone", pr.Number)
case ctx.prsKnown:
res.verdict, res.reason = verdictRemove, "no PR"
default:
res.verdict, res.reason = verdictRemove, "PR state unknown"
}
if res.verdict == verdictRemove && local > 0 {
res.reason += fmt.Sprintf(", %s on no remote so branch %s is kept", commitCount(local), wt.branch)
}
return res, nil
}
func commitCount(n int) string {
if n == 1 {
return "1 commit"
}
return fmt.Sprintf("%d commits", n)
}
File diff suppressed because it is too large Load Diff
+35 -11
View File
@@ -7,6 +7,7 @@
// watchpr owner/repo#12 owner/repo:15
// watchpr --once --json owner/repo#12
// watchpr --interval 30s owner/repo#12
// watchpr --interval 30 owner/repo#12
package main
import (
@@ -34,7 +35,7 @@ func main() {
// tests can invoke Execute and assert the exit behaviour without spawning a
// process.
func newRootCmd() *cobra.Command {
var interval time.Duration
var intervalFlag string
var once, jsonMode bool
root := &cobra.Command{
@@ -50,6 +51,10 @@ func newRootCmd() *cobra.Command {
if len(args) == 0 {
return fmt.Errorf("no PR references given (e.g. owner/repo#12)")
}
interval, err := agent.ParseDurationFlag("interval", intervalFlag)
if err != nil {
return err
}
refs := make([]agent.PRRef, 0, len(args))
for _, a := range args {
ref, err := agent.ParsePRRef(a)
@@ -58,10 +63,7 @@ func newRootCmd() *cobra.Command {
}
refs = append(refs, ref)
}
c, err := clientFor()
if err != nil {
return err
}
c := clientFor()
if once {
return runOnce(c, refs, jsonMode)
}
@@ -71,7 +73,7 @@ func newRootCmd() *cobra.Command {
root.SetVersionTemplate("{{.Version}}\n")
f := root.Flags()
f.DurationVar(&interval, "interval", 60*time.Second, "Polling interval")
f.StringVar(&intervalFlag, "interval", "60s", "Polling interval: a duration (30s, 2m, 1h30m) or a bare number of seconds")
f.BoolVar(&once, "once", false, "Check once, print current state, and exit")
f.BoolVar(&jsonMode, "json", false, "Emit JSON")
@@ -84,12 +86,16 @@ func newRootCmd() *cobra.Command {
return root
}
func clientFor() (*agent.GiteaClient, error) {
// 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 {
token, err := agent.GiteaToken()
if err != nil {
return nil, err
fmt.Fprintf(os.Stderr, "warning: no Gitea token (%v); polling anonymously\n", err)
token = ""
}
return agent.NewGiteaClient(token), nil
return agent.NewGiteaClient(token)
}
// runOnce fetches and prints the current state of each PR, then exits 0.
@@ -99,7 +105,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)
}
@@ -131,12 +137,30 @@ func runWatch(c *agent.GiteaClient, refs []agent.PRRef, interval time.Duration,
res, err := agent.Watch(c, refs, login, ticker.C, onBaseline, onError)
if err != nil {
return err
return describeFailure(err)
}
report(res.Ref.String(), res.Reason, res.State, jsonMode)
return nil
}
// 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 {
+154
View File
@@ -1,8 +1,15 @@
package main
import (
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"git.unkin.net/unkin/agent-tools/internal/agent"
)
// A bad PR reference must fail the command (so main exits non-zero) rather than
@@ -28,3 +35,150 @@ func TestExecuteNoArgsErrors(t *testing.T) {
t.Fatal("Execute() = nil, want error when no PR references are given")
}
}
// Watching a public repo with no credentials available must work: the failed
// mint is a warning, the poll goes out unauthenticated, and the command exits 0.
func TestOnceRunsAnonymouslyWhenNoTokenIsAvailable(t *testing.T) {
vault := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
}))
defer vault.Close()
authHeaders := 0
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
if r.Header.Get("Authorization") != "" {
authHeaders++
}
_, _ = 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, `[]`)
})
gitea := httptest.NewServer(mux)
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)
if err := cmd.Execute(); err != nil {
t.Fatalf("anonymous --once should succeed without a token: %v", err)
}
if authHeaders != 0 {
t.Errorf("sent %d Authorization headers, want none", authHeaders)
}
}
// A bare integer interval means seconds and must survive flag parsing: the
// command should fail on the missing PR reference, not on the flag value.
func TestExecuteBareIntervalIsSeconds(t *testing.T) {
cmd := newRootCmd()
cmd.SetArgs([]string{"--interval", "15"})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if err == nil {
t.Fatal("Execute() = nil, want the no-references error")
}
if !strings.Contains(err.Error(), "no PR references given") {
t.Fatalf("Execute() error = %v, want the no-references error", err)
}
}
// An unparseable interval is rejected before any Vault/Gitea call, with an
// error naming the flag and showing valid forms.
func TestExecuteBadIntervalErrors(t *testing.T) {
cmd := newRootCmd()
cmd.SetArgs([]string{"--interval", "soon", "unkin/repo#1"})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if err == nil {
t.Fatal("Execute() = nil, want error for an unparseable --interval")
}
for _, want := range []string{"--interval", "30s"} {
if !strings.Contains(err.Error(), want) {
t.Errorf("Execute() error %q does not mention %q", err, want)
}
}
}
// 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)
}
})
}
}
+856
View File
@@ -2,10 +2,14 @@ package agent
import (
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
)
// fakeVault serves the AppRole login and the gitea creds secret at credsPath
@@ -95,6 +99,80 @@ func TestCreatePRRequestBody(t *testing.T) {
}
}
// An edit must send only the fields it was given: Gitea overwrites whatever
// key it receives, so an omitted --title arriving as "" would blank the title.
func TestEditPRSendsOnlySuppliedFields(t *testing.T) {
title, body, empty := "new title", "new body", ""
tests := []struct {
name string
opts EditOptions
want map[string]any
}{
{"body only", EditOptions{Body: &body}, map[string]any{"body": "new body"}},
{"title only", EditOptions{Title: &title}, map[string]any{"title": "new title"}},
{"both", EditOptions{Title: &title, Body: &body}, map[string]any{"title": "new title", "body": "new body"}},
{"explicit empty body is sent", EditOptions{Body: &empty}, map[string]any{"body": ""}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var gotBody map[string]any
var gotMethod, gotPath string
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
gotMethod, gotPath = r.Method, r.URL.Path
_ = json.NewDecoder(r.Body).Decode(&gotBody)
_, _ = io.WriteString(w, `{"number":7,"title":"new title","html_url":"https://git.unkin.net/unkin/repo/pulls/7"}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "gitea-abc", HTTP: srv.Client()}
pr, err := c.EditPR("unkin/repo", 7, tt.opts)
if err != nil {
t.Fatalf("EditPR: %v", err)
}
if gotMethod != http.MethodPatch {
t.Errorf("method = %s, want PATCH", gotMethod)
}
if gotPath != "/api/v1/repos/unkin/repo/pulls/7" {
t.Errorf("path = %q", gotPath)
}
if len(gotBody) != len(tt.want) {
t.Errorf("payload = %v, want exactly the supplied fields %v", gotBody, tt.want)
}
for k, v := range tt.want {
if gotBody[k] != v {
t.Errorf("payload[%q] = %v, want %v", k, gotBody[k], v)
}
}
if pr.Number != 7 || pr.HTMLURL == "" {
t.Errorf("parsed PR = %+v", pr)
}
})
}
}
// A non-2xx must surface the API's own message rather than a bare status.
func TestEditPRAPIError(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = io.WriteString(w, `{"message":"not found","url":"https://git.unkin.net/api/swagger","errors":null}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
title := "new title"
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
_, err := c.EditPR("unkin/repo", 7, EditOptions{Title: &title})
if err == nil {
t.Fatal("expected error on 404")
}
if !strings.Contains(err.Error(), `"message":"not found"`) {
t.Errorf("error %q should carry the API message", err)
}
}
func TestCreateComment(t *testing.T) {
var gotBody map[string]string
mux := http.NewServeMux()
@@ -217,6 +295,100 @@ func TestFetchStateFailsOnNon404StatusError(t *testing.T) {
}
}
// Gitea rewrites head.ref to "refs/pull/<n>/head" once the PR's branch is
// deleted, which merging does in these repos. Matching a branch against
// head.ref alone therefore finds nothing for every merged PR; head.label keeps
// the original name.
func TestPRHeadBranch(t *testing.T) {
tests := []struct {
name string
ref, label string
want string
}{
{"merged, branch deleted", "refs/pull/12/head", "benvin/merged", "benvin/merged"},
{"open PR", "benvin/open", "benvin/open", "benvin/open"},
{"fully qualified ref", "refs/heads/benvin/x", "", "benvin/x"},
{"no label falls back to ref", "benvin/y", "", "benvin/y"},
{"cross-repo label", "benvin/z", "someone:benvin/z", "benvin/z"},
{"nothing usable", "refs/pull/12/head", "", ""},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var pr PullRequest
pr.Head.Ref = tt.ref
pr.Head.Label = tt.label
if got := PRHeadBranch(pr); got != tt.want {
t.Errorf("PRHeadBranch(ref=%q,label=%q) = %q, want %q", tt.ref, tt.label, got, tt.want)
}
})
}
}
func TestListPRsPaginates(t *testing.T) {
var pages []string
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls", func(w http.ResponseWriter, r *http.Request) {
q := r.URL.Query()
pages = append(pages, q.Get("page"))
if q.Get("state") != "all" {
t.Errorf("state = %q, want all", q.Get("state"))
}
if q.Get("page") == "1" {
full := make([]string, 0, prPageSize)
for i := 0; i < prPageSize; i++ {
full = append(full, fmt.Sprintf(`{"number":%d,"state":"closed","merged":true,"head":{"ref":"refs/pull/%d/head","label":"benvin/b%d"}}`, i+1, i+1, i+1))
}
_, _ = io.WriteString(w, "["+strings.Join(full, ",")+"]")
return
}
_, _ = io.WriteString(w, `[{"number":99,"state":"open","head":{"ref":"benvin/last","label":"benvin/last"}}]`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, HTTP: srv.Client()}
prs, err := c.ListPRs("unkin/repo", "all")
if err != nil {
t.Fatalf("ListPRs: %v", err)
}
if len(prs) != prPageSize+1 {
t.Fatalf("got %d PRs, want %d", len(prs), prPageSize+1)
}
if len(pages) != 2 || pages[0] != "1" || pages[1] != "2" {
t.Errorf("pages requested = %v, want [1 2]", pages)
}
if got := PRHeadBranch(prs[0]); got != "benvin/b1" {
t.Errorf("first PR head branch = %q, want benvin/b1", got)
}
if !prs[len(prs)-1].IsOpen() {
t.Error("last PR should be open")
}
}
// A listing that fills every page is truncated: the caller must be told rather
// than treating a partial view as the whole repo.
func TestListPRsReportsTruncation(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls", func(w http.ResponseWriter, r *http.Request) {
full := make([]string, 0, prPageSize)
for i := 0; i < prPageSize; i++ {
full = append(full, fmt.Sprintf(`{"number":%s,"state":"open"}`, r.URL.Query().Get("page")))
}
_, _ = io.WriteString(w, "["+strings.Join(full, ",")+"]")
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, HTTP: srv.Client()}
prs, err := c.ListPRs("unkin/repo", "all")
if !errors.Is(err, ErrPRListTruncated) {
t.Fatalf("ListPRs err = %v, want ErrPRListTruncated", err)
}
if len(prs) != maxPRPages*prPageSize {
t.Errorf("got %d PRs, want %d", len(prs), maxPRPages*prPageSize)
}
}
func TestGiteaAPIError(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls", func(w http.ResponseWriter, r *http.Request) {
@@ -231,3 +403,687 @@ func TestGiteaAPIError(t *testing.T) {
t.Fatal("expected error on 422")
}
}
// expiringGitea serves the PR endpoint, rejecting every token other than
// wantToken with a 401 exactly as Gitea does once a Vault-minted token expires.
// It records the tokens it saw, newest last.
func expiringGitea(t *testing.T, wantToken string, seen *[]string) *httptest.Server {
t.Helper()
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 != wantToken {
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"}}`)
})
return httptest.NewServer(mux)
}
// The production failure: the token expired mid-run. The client must re-mint
// once and replay the request with the fresh token.
func TestExpiredTokenIsRemintedAndRetried(t *testing.T) {
var seen []string
srv := expiringGitea(t, "fresh", &seen)
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 {
t.Errorf("PR number = %d, want 7", pr.Number)
}
if refreshes != 1 {
t.Errorf("refreshes = %d, want 1", refreshes)
}
if len(seen) != 2 || seen[0] != "stale" || seen[1] != "fresh" {
t.Errorf("tokens seen = %v, want [stale fresh]", seen)
}
if c.Token != "fresh" {
t.Errorf("client token = %q, want the refreshed token", c.Token)
}
}
// A fresh token that is also rejected is a real auth failure: report it as one
// rather than re-minting forever.
func TestAuthFailureSurvivesRemint(t *testing.T) {
var seen []string
srv := expiringGitea(t, "never-issued", &seen)
defer srv.Close()
refreshes := 0
c := &GiteaClient{BaseURL: srv.URL, Token: "stale", HTTP: srv.Client(),
Refresh: func() (string, error) { refreshes++; return "still-bad", nil }}
_, err := c.GetPR("unkin/repo", 7)
if err == nil {
t.Fatal("GetPR should fail when the fresh token is rejected too")
}
if !IsAuthError(err) {
t.Errorf("IsAuthError(%v) = false, want true", err)
}
if refreshes != 1 {
t.Errorf("refreshes = %d, want 1 (re-mint exactly once)", refreshes)
}
if len(seen) != 2 {
t.Errorf("requests = %d, want 2", len(seen))
}
}
// A refresh that itself fails must surface as an auth error, not as a silent
// success or a bare Vault error.
func TestRemintErrorIsReportedAsAuthFailure(t *testing.T) {
var seen []string
srv := expiringGitea(t, "fresh", &seen)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "stale", HTTP: srv.Client(),
Refresh: func() (string, error) { return "", errors.New("vault approle login: HTTP 503") }}
_, err := c.GetPR("unkin/repo", 7)
if err == nil || !IsAuthError(err) {
t.Fatalf("GetPR error = %v, want an auth error", err)
}
if !strings.Contains(err.Error(), "vault approle login") {
t.Errorf("error %q should name the re-mint failure", err)
}
if len(seen) != 1 {
t.Errorf("requests = %d, want 1 (no replay without a token)", len(seen))
}
}
// A 5xx is transient, not an auth problem: no re-mint, no retry, and the caller
// keeps its existing retry behaviour.
func TestServerErrorDoesNotRemint(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.StatusBadGateway)
})
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 error on 502")
}
if IsAuthError(err) {
t.Errorf("502 must not be an auth error")
}
if refreshes != 0 || requests != 1 {
t.Errorf("refreshes = %d, requests = %d, want 0 and 1", refreshes, requests)
}
}
// The replayed request must carry the original body, not an empty one.
func TestRemintReplaysRequestBody(t *testing.T) {
var bodies []CreatePROptions
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls", func(w http.ResponseWriter, r *http.Request) {
var body CreatePROptions
_ = json.NewDecoder(r.Body).Decode(&body)
bodies = append(bodies, body)
if strings.TrimPrefix(r.Header.Get("Authorization"), "token ") != "fresh" {
w.WriteHeader(http.StatusUnauthorized)
return
}
_, _ = io.WriteString(w, `{"number":7}`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "stale", HTTP: srv.Client(),
Refresh: func() (string, error) { return "fresh", nil }}
if _, err := c.CreatePR("unkin/repo", CreatePROptions{Base: "main", Head: "feature", Title: "T", Body: "B"}); err != nil {
t.Fatalf("CreatePR: %v", err)
}
if len(bodies) != 2 {
t.Fatalf("requests = %d, want 2", len(bodies))
}
if bodies[1] != bodies[0] {
t.Errorf("replayed body = %+v, want %+v", bodies[1], bodies[0])
}
}
func TestIsAuthError(t *testing.T) {
tests := []struct {
status int
want bool
}{
{http.StatusUnauthorized, true},
{http.StatusForbidden, true},
{http.StatusNotFound, false},
{http.StatusUnprocessableEntity, false},
{http.StatusBadGateway, false},
}
for _, tt := range tests {
err := error(&APIError{Method: "GET", Path: "/p", StatusCode: tt.status})
if got := IsAuthError(err); got != tt.want {
t.Errorf("IsAuthError(HTTP %d) = %v, want %v", tt.status, got, tt.want)
}
}
if IsAuthError(errors.New("dial tcp: timeout")) {
t.Errorf("a network error is not an auth error")
}
}
// Anonymous polling of a public repo is a supported mode: with no token the
// client must send no Authorization header, and must never reach for Vault.
func TestAnonymousPollingNeverMints(t *testing.T) {
authHeaders := 0
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
if r.Header.Get("Authorization") != "" {
authHeaders++
}
_, _ = 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, HTTP: srv.Client(),
Refresh: func() (string, error) { refreshes++; return "", errors.New("vault unreachable") }}
st, err := FetchState(c, PRRef{Owner: "unkin", Repo: "repo", Number: 7}, "unkin-agent")
if err != nil {
t.Fatalf("anonymous FetchState: %v", err)
}
if st.State != "open" || st.CIStatus != "success" || st.HeadSHA != "cafebabe" {
t.Errorf("state = %+v", st)
}
if refreshes != 0 {
t.Errorf("refreshes = %d, want 0 (a 200 must never trigger a mint)", refreshes)
}
if authHeaders != 0 {
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)
}
}
+163
View File
@@ -2,10 +2,12 @@ package agent
import (
"bytes"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
)
@@ -24,6 +26,10 @@ type Worktree struct {
Branch string // short branch name ("" when detached or bare)
Bare bool
Detached bool
Locked bool
// Prunable is git's own reason a registration is stale (e.g. "gitdir file
// points to non-existent location"); empty when the worktree is intact.
Prunable string
}
// runGit runs git with args, using dir as the working directory (empty = the
@@ -67,6 +73,14 @@ func GitFetch(repoDir, remote string, globalArgs ...string) error {
return err
}
// GitFetchPrune runs `git fetch --prune <remote>` in repoDir so remote-tracking
// refs for branches deleted on the remote (e.g. after a merge) disappear.
func GitFetchPrune(repoDir, remote string, globalArgs ...string) error {
args := append(append([]string{}, globalArgs...), "fetch", "--prune", remote)
_, err := runGit(repoDir, args...)
return err
}
// GitRemoteDefaultBranch returns the short name of remote's default branch
// (e.g. "main") by resolving refs/remotes/<remote>/HEAD.
func GitRemoteDefaultBranch(repoDir, remote string) (string, error) {
@@ -83,6 +97,143 @@ func GitBranchExists(repoDir, branch string) bool {
return err == nil
}
// GitRemoteURL returns the configured URL for a remote.
func GitRemoteURL(repoDir, remote string) (string, error) {
return runGit(repoDir, "remote", "get-url", remote)
}
// GitRemoteBranchExists reports whether a remote-tracking ref for branch exists
// (accurate only after a pruning fetch).
func GitRemoteBranchExists(repoDir, remote, branch string) bool {
_, err := runGit(repoDir, "show-ref", "--verify", "--quiet", "refs/remotes/"+remote+"/"+branch)
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) {
out, err := runGit(dir, "status", "--porcelain")
if err != nil {
return false, err
}
return strings.TrimSpace(out) != "", nil
}
// GitDir returns the absolute path to the git directory backing the checkout at
// dir (per-worktree, unlike GitCommonDir).
func GitDir(dir string) (string, error) {
return runGit(dir, "rev-parse", "--path-format=absolute", "--git-dir")
}
// inProgressMarkers maps a sentinel inside the git dir to the operation it means
// is half-finished. Such a checkout holds state that lives nowhere else.
var inProgressMarkers = []struct{ path, op string }{
{"rebase-merge", "rebase"},
{"rebase-apply", "rebase"},
{"MERGE_HEAD", "merge"},
{"CHERRY_PICK_HEAD", "cherry-pick"},
{"REVERT_HEAD", "revert"},
{"BISECT_LOG", "bisect"},
}
// GitInProgressOp names the sequencer operation underway in the checkout at dir,
// or "" when none is.
func GitInProgressOp(dir string) (string, error) {
gitDir, err := GitDir(dir)
if err != nil {
return "", err
}
for _, m := range inProgressMarkers {
if _, err := os.Stat(filepath.Join(gitDir, m.path)); err == nil {
return m.op, nil
} else if !os.IsNotExist(err) {
return "", err
}
}
return "", nil
}
// GitCommitsNotOnRemotes counts commits reachable from HEAD that no
// remote-tracking ref holds, i.e. work that exists only in this checkout.
func GitCommitsNotOnRemotes(dir string) (int, error) {
out, err := runGit(dir, "rev-list", "--count", "HEAD", "--not", "--remotes")
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
}
// GitIsAncestor reports whether ancestor is reachable from descendant.
func GitIsAncestor(repoDir, ancestor, descendant string) (bool, error) {
cmd := exec.Command("git", "merge-base", "--is-ancestor", ancestor, descendant)
cmd.Dir = repoDir
var stderr bytes.Buffer
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
// Exit 1 is the documented "not an ancestor" answer; anything else is a
// real failure (bad ref, not a repo).
var exitErr *exec.ExitError
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
return false, nil
}
return false, fmt.Errorf("git merge-base --is-ancestor %s %s: %w: %s",
ancestor, descendant, err, strings.TrimSpace(stderr.String()))
}
return true, nil
}
// GitUnmergedCommits counts commits on head whose patch has no equivalent on
// upstream, using `git cherry` so squash- and rebase-merged work is recognised
// despite its rewritten SHAs.
func GitUnmergedCommits(repoDir, upstream, head string) (int, error) {
out, err := runGit(repoDir, "cherry", upstream, head)
if err != nil {
return 0, err
}
n := 0
for _, line := range strings.Split(out, "\n") {
if strings.HasPrefix(strings.TrimSpace(line), "+") {
n++
}
}
return n, nil
}
// GitWorktreeAdd adds a worktree at path checked out to branch. When the branch
// already exists it is reused; otherwise it is created from startPoint.
func GitWorktreeAdd(repoDir, path, branch, startPoint string) error {
@@ -210,6 +361,18 @@ func ParseWorktreeList(out string) []Worktree {
if cur != nil {
cur.Detached = true
}
case "locked":
if cur != nil {
cur.Locked = true
}
case "prunable":
if cur != nil {
// git omits the reason when it has none, so record the flag itself.
cur.Prunable = val
if cur.Prunable == "" {
cur.Prunable = "prunable"
}
}
}
}
flush()
+238 -2
View File
@@ -42,10 +42,20 @@ branch refs/heads/benvin/foo
worktree /home/ben/.cache/agentws/repo__detached
HEAD 3333333333333333333333333333333333333333
detached
worktree /home/ben/.cache/agentws/repo__gone
HEAD 4444444444444444444444444444444444444444
branch refs/heads/benvin/gone
prunable gitdir file points to non-existent location
worktree /home/ben/.cache/agentws/repo__held
HEAD 5555555555555555555555555555555555555555
branch refs/heads/benvin/held
locked
`
wts := ParseWorktreeList(out)
if len(wts) != 3 {
t.Fatalf("got %d worktrees, want 3: %+v", len(wts), wts)
if len(wts) != 5 {
t.Fatalf("got %d worktrees, want 5: %+v", len(wts), wts)
}
if wts[0].Branch != "main" || wts[0].Path != "/home/ben/src/prodenv/repo" {
t.Errorf("wt[0] = %+v", wts[0])
@@ -56,6 +66,104 @@ detached
if !wts[2].Detached || wts[2].Branch != "" {
t.Errorf("wt[2] = %+v, want detached with empty branch", wts[2])
}
if wts[3].Prunable != "gitdir file points to non-existent location" {
t.Errorf("wt[3].Prunable = %q", wts[3].Prunable)
}
if !wts[4].Locked || wts[4].Prunable != "" {
t.Errorf("wt[4] = %+v, want locked and not prunable", wts[4])
}
}
// A bare "prunable" with no reason still has to read as prunable.
func TestParseWorktreeListPrunableWithoutReason(t *testing.T) {
wts := ParseWorktreeList("worktree /tmp/wt\nHEAD 1111111111111111111111111111111111111111\ndetached\nprunable\n")
if len(wts) != 1 || wts[0].Prunable == "" {
t.Errorf("ParseWorktreeList = %+v, want one prunable worktree", wts)
}
}
func TestGitInProgressOp(t *testing.T) {
srcDir := newTempRepos(t)
gitDir, err := GitDir(srcDir)
if err != nil {
t.Fatalf("GitDir: %v", err)
}
if op, err := GitInProgressOp(srcDir); err != nil || op != "" {
t.Fatalf("clean checkout: op = %q, err = %v", op, err)
}
tests := []struct {
marker string
dir bool
want string
}{
{"MERGE_HEAD", false, "merge"},
{"CHERRY_PICK_HEAD", false, "cherry-pick"},
{"REVERT_HEAD", false, "revert"},
{"BISECT_LOG", false, "bisect"},
{"rebase-merge", true, "rebase"},
{"rebase-apply", true, "rebase"},
}
for _, tt := range tests {
path := filepath.Join(gitDir, tt.marker)
if tt.dir {
if err := os.Mkdir(path, 0o755); err != nil {
t.Fatal(err)
}
} else if err := os.WriteFile(path, []byte("x\n"), 0o644); err != nil {
t.Fatal(err)
}
op, err := GitInProgressOp(srcDir)
if err != nil || op != tt.want {
t.Errorf("%s: op = %q, err = %v; want %q", tt.marker, op, err, tt.want)
}
if err := os.RemoveAll(path); err != nil {
t.Fatal(err)
}
}
}
// Commits that reached a remote-tracking ref are not unique local work; commits
// made after the push are.
func TestGitCommitsNotOnRemotes(t *testing.T) {
srcDir := newTempRepos(t)
if n, err := GitCommitsNotOnRemotes(srcDir); err != nil || n != 0 {
t.Fatalf("freshly cloned main: n = %d, err = %v; want 0", n, err)
}
if _, err := runGit(srcDir, "checkout", "-b", "benvin/x"); err != nil {
t.Fatal(err)
}
commitFile(t, srcDir, "a.txt", "a\n", "local a")
commitFile(t, srcDir, "b.txt", "b\n", "local b")
if n, err := GitCommitsNotOnRemotes(srcDir); err != nil || n != 2 {
t.Fatalf("two unpushed commits: n = %d, err = %v; want 2", n, err)
}
if _, err := runGit(srcDir, "push", "origin", "benvin/x"); err != nil {
t.Fatal(err)
}
if n, err := GitCommitsNotOnRemotes(srcDir); err != nil || n != 0 {
t.Fatalf("after push: n = %d, err = %v; want 0", n, err)
}
commitFile(t, srcDir, "c.txt", "c\n", "local c")
if n, err := GitCommitsNotOnRemotes(srcDir); err != nil || n != 1 {
t.Fatalf("one commit past the push: n = %d, err = %v; want 1", n, err)
}
}
func commitFile(t *testing.T, dir, name, content, msg string) {
t.Helper()
if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
t.Fatal(err)
}
if _, err := runGit(dir, "add", "."); err != nil {
t.Fatal(err)
}
if _, err := runGit(dir, "commit", "-m", msg); err != nil {
t.Fatal(err)
}
}
// gitSeed sets a repo-local identity so commits work without global config.
@@ -185,6 +293,134 @@ func TestGitWorktreeLifecycle(t *testing.T) {
}
}
// commit writes a file and commits it, returning the new HEAD sha.
func commit(t *testing.T, dir, name, content, msg string) string {
t.Helper()
if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
t.Fatal(err)
}
if _, err := runGit(dir, "add", "."); err != nil {
t.Fatalf("add: %v", err)
}
if _, err := runGit(dir, "commit", "-m", msg); err != nil {
t.Fatalf("commit: %v", err)
}
sha, err := runGit(dir, "rev-parse", "HEAD")
if err != nil {
t.Fatalf("rev-parse: %v", err)
}
return sha
}
func TestGitIsAncestor(t *testing.T) {
srcDir := newTempRepos(t)
base, err := runGit(srcDir, "rev-parse", "HEAD")
if err != nil {
t.Fatal(err)
}
tip := commit(t, srcDir, "a.txt", "a\n", "add a")
if ok, err := GitIsAncestor(srcDir, base, tip); err != nil || !ok {
t.Errorf("GitIsAncestor(base, tip) = %v, %v; want true", ok, err)
}
if ok, err := GitIsAncestor(srcDir, tip, base); err != nil || ok {
t.Errorf("GitIsAncestor(tip, base) = %v, %v; want false with no error", ok, err)
}
if _, err := GitIsAncestor(srcDir, "no-such-ref", tip); err == nil {
t.Error("GitIsAncestor with a bogus ref should error, not report false")
}
}
// These repos squash-merge, so merged work keeps its local SHA while the
// upstream commit is a different one carrying the same patch. `git cherry` must
// see that as merged even though the SHAs differ.
func TestGitUnmergedCommitsIgnoresRewrittenSHAs(t *testing.T) {
srcDir := newTempRepos(t)
if _, err := runGit(srcDir, "checkout", "-b", "feature"); err != nil {
t.Fatalf("checkout: %v", err)
}
commit(t, srcDir, "f.txt", "hello\n", "add f")
n, err := GitUnmergedCommits(srcDir, "origin/main", "HEAD")
if err != nil {
t.Fatalf("GitUnmergedCommits: %v", err)
}
if n != 1 {
t.Fatalf("unmerged before upstream landing = %d, want 1", n)
}
// Land the same patch upstream under a different SHA.
if _, err := runGit(srcDir, "checkout", "main"); err != nil {
t.Fatalf("checkout main: %v", err)
}
commit(t, srcDir, "f.txt", "hello\n", "squashed f")
if _, err := runGit(srcDir, "push", "origin", "main"); err != nil {
t.Fatalf("push: %v", err)
}
if err := GitFetchPrune(srcDir, "origin"); err != nil {
t.Fatalf("GitFetchPrune: %v", err)
}
if ok, err := GitIsAncestor(srcDir, "feature", "origin/main"); err != nil || ok {
t.Fatalf("squash-merged branch must not be an ancestor: %v, %v", ok, err)
}
n, err = GitUnmergedCommits(srcDir, "origin/main", "feature")
if err != nil {
t.Fatalf("GitUnmergedCommits: %v", err)
}
if n != 0 {
t.Errorf("unmerged after upstream landing = %d, want 0", n)
}
}
func TestGitIsDirty(t *testing.T) {
srcDir := newTempRepos(t)
if dirty, err := GitIsDirty(srcDir); err != nil || dirty {
t.Fatalf("clean checkout reported dirty=%v, err=%v", dirty, err)
}
if err := os.WriteFile(filepath.Join(srcDir, "scratch.txt"), []byte("wip\n"), 0o644); err != nil {
t.Fatal(err)
}
if dirty, err := GitIsDirty(srcDir); err != nil || !dirty {
t.Errorf("untracked file must count as dirty: dirty=%v, err=%v", dirty, err)
}
}
func TestGitRemoteBranchExists(t *testing.T) {
srcDir := newTempRepos(t)
if !GitRemoteBranchExists(srcDir, "origin", "main") {
t.Error("origin/main should exist")
}
if GitRemoteBranchExists(srcDir, "origin", "benvin/nope") {
t.Error("origin/benvin/nope should not exist")
}
if _, err := runGit(srcDir, "checkout", "-b", "benvin/pushed"); err != nil {
t.Fatalf("checkout: %v", err)
}
commit(t, srcDir, "p.txt", "p\n", "add p")
if _, err := runGit(srcDir, "push", "origin", "benvin/pushed"); err != nil {
t.Fatalf("push: %v", err)
}
if err := GitFetchPrune(srcDir, "origin"); err != nil {
t.Fatalf("GitFetchPrune: %v", err)
}
if !GitRemoteBranchExists(srcDir, "origin", "benvin/pushed") {
t.Error("pushed branch should have a remote-tracking ref")
}
if _, err := runGit(srcDir, "push", "origin", "--delete", "benvin/pushed"); err != nil {
t.Fatalf("delete remote branch: %v", err)
}
if err := GitFetchPrune(srcDir, "origin"); err != nil {
t.Fatalf("GitFetchPrune: %v", err)
}
if GitRemoteBranchExists(srcDir, "origin", "benvin/pushed") {
t.Error("a pruning fetch must drop the tracking ref for a deleted remote branch")
}
}
// resolve canonicalizes a path (temp dirs may live behind symlinks like /var).
func resolve(t *testing.T, p string) string {
t.Helper()
+253 -10
View File
@@ -24,40 +24,134 @@ func (e *APIError) Error() string {
return fmt.Sprintf("gitea %s %s: HTTP %d: %s", e.Method, e.Path, e.StatusCode, e.Body)
}
// isNotFound reports whether err is a Gitea 404.
func isNotFound(err error) bool {
// IsNotFound reports whether err is a Gitea 404. Gitea hides repositories a
// caller may not see behind a 404 rather than a 403, so this also covers a repo
// that was renamed, deleted, or made private.
func IsNotFound(err error) bool {
var apiErr *APIError
return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
}
// 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
Token string
HTTP *http.Client
// Refresh mints a replacement token when the current one is rejected; Vault's
// Gitea tokens expire in ~1h, far short of a watchpr run.
Refresh func() (string, error)
}
// NewGiteaClient builds a client from the configured base URL and a Vault-minted
// token.
// token, re-minting from Vault when that token expires.
func NewGiteaClient(token string) *GiteaClient {
return &GiteaClient{BaseURL: GiteaURL(), Token: token, HTTP: httpClient}
return &GiteaClient{BaseURL: GiteaURL(), Token: token, HTTP: httpClient, Refresh: RefreshGiteaToken}
}
// 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 reader io.Reader
var payload []byte
if body != nil {
b, err := json.Marshal(body)
if err != nil {
return err
}
reader = bytes.NewReader(b)
payload = b
}
anonymous := c.Token == ""
err := c.attempt(method, path, payload, out)
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)
}
func (c *GiteaClient) attempt(method, path string, body []byte, out any) error {
var reader io.Reader
if body != nil {
reader = bytes.NewReader(body)
}
url := strings.TrimRight(c.BaseURL, "/") + path
req, err := http.NewRequest(method, url, reader)
if err != nil {
return err
}
req.Header.Set("Authorization", "token "+c.Token)
// An empty token means anonymous access, which public repos serve fine.
if c.Token != "" {
req.Header.Set("Authorization", "token "+c.Token)
}
req.Header.Set("Accept", "application/json")
if body != nil {
req.Header.Set("Content-Type", "application/json")
@@ -102,10 +196,68 @@ type PullRequest struct {
Mergeable bool `json:"mergeable"`
HTMLURL string `json:"html_url"`
Head struct {
Sha string `json:"sha"`
Sha string `json:"sha"`
Ref string `json:"ref"`
Label string `json:"label"`
} `json:"head"`
}
// prPageSize is the per-page limit for the pulls listing; maxPRPages caps how
// far back a listing walks.
const (
prPageSize = 50
maxPRPages = 20
)
// ErrPRListTruncated reports that a listing hit the page cap, so the returned
// pull requests are only the most recent ones and older PRs went unseen.
var ErrPRListTruncated = errors.New("pull request listing truncated at the page cap")
// ListPRs lists a repo's pull requests in the given state ("open", "closed" or
// "all"), following pagination. A repo with more PRs than the page cap returns
// the PRs it did read alongside ErrPRListTruncated.
func (c *GiteaClient) ListPRs(repoPath, state string) ([]PullRequest, error) {
if state == "" {
state = "all"
}
var all []PullRequest
for page := 1; page <= maxPRPages; page++ {
var batch []PullRequest
path := fmt.Sprintf("/api/v1/repos/%s/pulls?state=%s&limit=%d&page=%d", repoPath, state, prPageSize, page)
if err := c.do(http.MethodGet, path, nil, &batch); err != nil {
return nil, err
}
all = append(all, batch...)
if len(batch) < prPageSize {
return all, nil
}
}
return all, fmt.Errorf("%s: %w after %d pull requests", repoPath, ErrPRListTruncated, len(all))
}
// PRHeadBranch returns the branch a PR was opened from. Gitea rewrites head.ref
// to "refs/pull/<n>/head" once the branch is deleted (which merging does), so
// head.label — which keeps the original name — is authoritative.
func PRHeadBranch(pr PullRequest) string {
if label := pr.Head.Label; label != "" && !strings.HasPrefix(label, "refs/pull/") {
// Cross-repo PRs label as "<owner>:<branch>".
if _, branch, ok := strings.Cut(label, ":"); ok {
return branch
}
return label
}
ref := pr.Head.Ref
if strings.HasPrefix(ref, "refs/pull/") {
return ""
}
return strings.TrimPrefix(ref, "refs/heads/")
}
// IsOpen reports whether a PR is still open (not merged, not closed).
func (pr PullRequest) IsOpen() bool {
return pr.State == "open" && !pr.Merged
}
// CreatePROptions are the fields for opening a PR.
type CreatePROptions struct {
Base string `json:"base"`
@@ -121,6 +273,25 @@ func (c *GiteaClient) CreatePR(repoPath string, opts CreatePROptions) (PullReque
return pr, err
}
// EditOptions are the fields an edit may change, for a pull request or an
// issue alike. Pointers so an unset field is omitted from the payload
// entirely, leaving that field as it is. The two fields are not symmetric:
// Gitea only applies a title when it is non-empty, so Title can be set but
// never cleared and a "" title is a silent no-op, while a pointer to "" Body
// really does blank the body.
type EditOptions struct {
Title *string `json:"title,omitempty"`
Body *string `json:"body,omitempty"`
}
// EditPR updates a pull request's title and/or body
// (PATCH /api/v1/repos/{owner}/{repo}/pulls/{index}).
func (c *GiteaClient) EditPR(repoPath string, number int, opts EditOptions) (PullRequest, error) {
var pr PullRequest
err := c.do(http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/pulls/%d", repoPath, number), opts, &pr)
return pr, err
}
// GetPR fetches a single pull request.
func (c *GiteaClient) GetPR(repoPath string, number int) (PullRequest, error) {
var pr PullRequest
@@ -128,6 +299,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"`
@@ -135,8 +376,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}
+66
View File
@@ -4,6 +4,7 @@ import (
"fmt"
"strconv"
"strings"
"time"
)
// PRRef identifies a single pull request by repository and number.
@@ -48,6 +49,71 @@ func ParsePRRef(s string) (PRRef, error) {
return PRRef{Owner: owner, Repo: repo, Number: n}, nil
}
// ParseDurationFlag parses a duration flag value, accepting either a Go
// duration string ("30s", "1h30m") or a bare integer read as seconds ("15").
// flag names the flag so the error says which value was rejected.
func ParseDurationFlag(flag, value string) (time.Duration, error) {
s := strings.TrimSpace(value)
d, err := time.ParseDuration(s)
if err != nil {
n, nerr := strconv.Atoi(s)
if nerr != nil {
return 0, fmt.Errorf("invalid --%s value %q: want a duration such as 30s, 2m or 1h30m, or a bare number of seconds such as 15", flag, value)
}
d = time.Duration(n) * time.Second
}
if d <= 0 {
return 0, fmt.Errorf("invalid --%s value %q: must be greater than zero", flag, value)
}
return d, nil
}
// RemoteHost returns the host a git remote URL points at, or "" for a local
// path remote.
func RemoteHost(remote string) string {
s := strings.TrimSpace(remote)
if _, after, ok := strings.Cut(s, "://"); ok {
host, _, _ := strings.Cut(after, "/")
if _, bare, ok := strings.Cut(host, "@"); ok {
host = bare
}
return host
}
if strings.HasPrefix(s, "/") || strings.HasPrefix(s, ".") {
return ""
}
host, _, ok := strings.Cut(s, ":")
if !ok {
return ""
}
if _, bare, ok := strings.Cut(host, "@"); ok {
host = bare
}
return host
}
// RepoPathFromRemoteURL extracts the "owner/repo" API path from a git remote
// URL, accepting both https and scp-style ssh forms.
func RepoPathFromRemoteURL(remote string) (string, error) {
s := strings.TrimSuffix(strings.TrimSuffix(strings.TrimSpace(remote), "/"), ".git")
switch {
case strings.Contains(s, "://"):
_, after, _ := strings.Cut(s, "://")
_, path, ok := strings.Cut(after, "/")
if !ok {
return "", fmt.Errorf("remote URL %q has no repo path", remote)
}
s = path
case strings.Contains(s, ":"):
_, s, _ = strings.Cut(s, ":")
}
parts := strings.Split(strings.Trim(s, "/"), "/")
if len(parts) < 2 || parts[len(parts)-2] == "" || parts[len(parts)-1] == "" {
return "", fmt.Errorf("remote URL %q is not owner/repo shaped", remote)
}
return parts[len(parts)-2] + "/" + parts[len(parts)-1], nil
}
// ParseRepo validates and splits an "owner/repo" string.
func ParseRepo(s string) (owner, repo string, err error) {
s = strings.TrimSpace(s)
+105 -1
View File
@@ -1,6 +1,10 @@
package agent
import "testing"
import (
"strings"
"testing"
"time"
)
func TestParsePRRef(t *testing.T) {
tests := []struct {
@@ -77,3 +81,103 @@ func TestParseRepo(t *testing.T) {
}
}
}
func TestParseDurationFlag(t *testing.T) {
tests := []struct {
in string
want time.Duration
wantErr bool
}{
{"15", 15 * time.Second, false},
{"15s", 15 * time.Second, false},
{"2m", 2 * time.Minute, false},
{"1h30m", 90 * time.Minute, false},
{"500ms", 500 * time.Millisecond, false},
{" 45 ", 45 * time.Second, false},
{"0", 0, true},
{"0s", 0, true},
{"-5", 0, true},
{"-5s", 0, true},
{"15x", 0, true},
{"", 0, true},
}
for _, tt := range tests {
got, err := ParseDurationFlag("interval", tt.in)
if tt.wantErr {
if err == nil {
t.Errorf("ParseDurationFlag(%q): expected error, got %v", tt.in, got)
}
continue
}
if err != nil {
t.Errorf("ParseDurationFlag(%q): unexpected error: %v", tt.in, err)
continue
}
if got != tt.want {
t.Errorf("ParseDurationFlag(%q) = %v, want %v", tt.in, got, tt.want)
}
}
}
// The error must name the flag and show valid forms instead of surfacing
// time.ParseDuration's "missing unit" wording.
func TestParseDurationFlagErrorMessage(t *testing.T) {
_, err := ParseDurationFlag("interval", "soon")
if err == nil {
t.Fatal("ParseDurationFlag(\"soon\"): expected error")
}
for _, want := range []string{"--interval", `"soon"`, "30s", "seconds"} {
if !strings.Contains(err.Error(), want) {
t.Errorf("error %q does not mention %q", err, want)
}
}
}
// Not every managed repo lives under the default owner, so the API path comes
// from origin's URL rather than the directory name.
func TestRepoPathFromRemoteURL(t *testing.T) {
tests := []struct {
in string
want string
}{
{"https://git.unkin.net/unkin/agent-tools.git", "unkin/agent-tools"},
{"https://git.unkin.net/unkinben/dotfiles.git", "unkinben/dotfiles"},
{"https://git.unkin.net/unkin/agent-tools", "unkin/agent-tools"},
{"https://user@git.unkin.net/unkin/agent-tools.git", "unkin/agent-tools"},
{"ssh://git@git.unkin.net:2222/unkin/agent-tools.git", "unkin/agent-tools"},
{"git@git.unkin.net:unkin/agent-tools.git", "unkin/agent-tools"},
}
for _, tt := range tests {
got, err := RepoPathFromRemoteURL(tt.in)
if err != nil {
t.Errorf("RepoPathFromRemoteURL(%q): %v", tt.in, err)
continue
}
if got != tt.want {
t.Errorf("RepoPathFromRemoteURL(%q) = %q, want %q", tt.in, got, tt.want)
}
}
for _, bad := range []string{"", "https://git.unkin.net", "agent-tools"} {
if got, err := RepoPathFromRemoteURL(bad); err == nil {
t.Errorf("RepoPathFromRemoteURL(%q) = %q, want error", bad, got)
}
}
}
func TestRemoteHost(t *testing.T) {
tests := []struct {
in, want string
}{
{"https://git.unkin.net/unkin/repo.git", "git.unkin.net"},
{"https://user@git.unkin.net/unkin/repo.git", "git.unkin.net"},
{"ssh://git@git.unkin.net:2222/unkin/repo.git", "git.unkin.net:2222"},
{"git@git.unkin.net:unkin/repo.git", "git.unkin.net"},
{"/tmp/fixture/origin.git", ""},
{"../other/origin.git", ""},
}
for _, tt := range tests {
if got := RemoteHost(tt.in); got != tt.want {
t.Errorf("RemoteHost(%q) = %q, want %q", tt.in, got, tt.want)
}
}
}
+19 -5
View File
@@ -86,17 +86,31 @@ func AuthentikURL() string {
}
var (
tokenOnce sync.Once
tokenValue string
tokenErr error
tokenMu sync.Mutex
tokenMinted bool
tokenValue string
tokenErr error
)
// GiteaToken returns a Gitea token, minting it via Vault AppRole on first call
// and caching it in-process for the lifetime of the command.
func GiteaToken() (string, error) {
tokenOnce.Do(func() {
tokenMu.Lock()
defer tokenMu.Unlock()
if !tokenMinted {
tokenValue, tokenErr = fetchGiteaToken(VaultAddr(), RoleID(), GiteaCredsPath())
})
tokenMinted = true
}
return tokenValue, tokenErr
}
// RefreshGiteaToken mints a fresh Gitea token and replaces the cached one, for
// callers that outlive the ~1h token TTL.
func RefreshGiteaToken() (string, error) {
tokenMu.Lock()
defer tokenMu.Unlock()
tokenValue, tokenErr = fetchGiteaToken(VaultAddr(), RoleID(), GiteaCredsPath())
tokenMinted = true
return tokenValue, tokenErr
}
+41 -5
View File
@@ -1,6 +1,20 @@
package agent
import "time"
import (
"errors"
"fmt"
"time"
)
// errPRGone marks a 404 from the PR lookup itself. A 404 from any other endpoint
// can be a proxy or ingress blip and is left to the ordinary failure cap.
var errPRGone = errors.New("PR no longer visible")
// IsPRGone reports whether err is a 404 from the PR lookup, meaning the PR is no
// longer visible rather than one endpoint being briefly unreachable.
func IsPRGone(err error) bool {
return errors.Is(err, errPRGone)
}
// PRState is a point-in-time snapshot of the PR attributes watchpr tracks.
type PRState struct {
@@ -20,13 +34,16 @@ type PRState struct {
func FetchState(c *GiteaClient, ref PRRef, agentLogin string) (PRState, error) {
pr, err := c.GetPR(ref.RepoPath(), ref.Number)
if err != nil {
if IsNotFound(err) {
return PRState{}, fmt.Errorf("%w: %w", errPRGone, err)
}
return PRState{}, err
}
// A 404 here means the head commit is gone (branch deleted after a squash/
// rebase merge); the PR object is still authoritative, so treat CI as absent
// rather than discarding the merge signal and hanging the watch loop.
ci, err := c.CommitStatus(ref.RepoPath(), pr.Head.Sha)
if err != nil && !isNotFound(err) {
if err != nil && !IsNotFound(err) {
return PRState{}, err
}
comments, err := c.ListComments(ref.RepoPath(), ref.Number)
@@ -78,12 +95,22 @@ func terminalState(st PRState) (bool, string) {
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.
const MaxPollFailures = 20
// Watch establishes a baseline for each ref, then polls on every tick until a
// tracked PR changes meaningfully, returning the first such change. A PR that is
// already terminal (merged/closed) at baseline is reported immediately rather
// than polled forever. Poll errors are handed to onError and never stop the
// loop; only a baseline fetch error aborts. onBaseline, if set, fires once after
// all baselines are captured and before the first tick.
// than polled forever. Transient poll errors are handed to onError and the loop
// continues, but never blindly: a baseline fetch error, an authentication
// failure surviving a token re-mint, a 404 from the PR lookup itself (the repo
// 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))
for _, ref := range refs {
@@ -99,16 +126,25 @@ func Watch(f StateFetcher, refs []PRRef, agentLogin string, ticks <-chan time.Ti
if onBaseline != nil {
onBaseline()
}
fails := make(map[string]int, len(refs))
for range ticks {
for _, ref := range refs {
key := ref.String()
cur, err := f.FetchState(ref, agentLogin)
if err != nil {
if IsAuthError(err) || IsPRGone(err) {
return WatchResult{}, fmt.Errorf("polling %s: %w", key, err)
}
fails[key]++
if onError != nil {
onError(ref, err)
}
if fails[key] >= MaxPollFailures {
return WatchResult{}, fmt.Errorf("polling %s: giving up after %d consecutive failures: %w", key, fails[key], err)
}
continue
}
fails[key] = 0
if changed, reason := MeaningfulChange(prev[key], cur); changed {
return WatchResult{Ref: ref, Reason: reason, State: cur}, nil
}
+441
View File
@@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"
"time"
@@ -360,3 +361,443 @@ func TestCountNonAgentComments(t *testing.T) {
t.Errorf("countNonAgentComments = %d, want 2", n)
}
}
// The production failure: the Vault-minted token expired mid-watch and every
// poll 401'd, which the loop logged as a warning and polled past forever. An
// auth error that survived the client's re-mint must end the watch with an
// error so watchpr exits non-zero instead of watching blind.
func TestWatchAbortsOnAuthError(t *testing.T) {
open := base()
merged := base()
merged.State = "closed"
merged.Merged = true
f := &fakeFetcher{
states: []PRState{open, open, merged},
errs: []error{nil, &APIError{Method: "GET", Path: "/p", StatusCode: 401, Body: "invalid token"}, nil},
}
warned := 0
ticks := make(chan time.Time, 2)
ticks <- time.Now()
ticks <- time.Now()
_, err := Watch(f, []PRRef{open.Ref}, "unkin-agent", ticks, nil, func(PRRef, error) { warned++ })
if err == nil {
t.Fatal("Watch should return the auth failure, not keep polling")
}
if !IsAuthError(err) {
t.Errorf("Watch error = %v, want an auth error", err)
}
if warned != 0 {
t.Errorf("auth failure was logged as a warning %d time(s); it must abort", warned)
}
if f.calls != 2 {
t.Errorf("fetch calls = %d, want 2 (baseline + the failing poll)", f.calls)
}
}
// A 5xx keeps its retry behaviour: warn and poll on.
func TestWatchContinuesPastServerError(t *testing.T) {
open := base()
merged := base()
merged.State = "closed"
merged.Merged = true
f := &fakeFetcher{
states: []PRState{open, open, merged},
errs: []error{nil, &APIError{Method: "GET", Path: "/p", StatusCode: 502, Body: "bad gateway"}, nil},
}
warned := 0
ticks := make(chan time.Time, 2)
ticks <- time.Now()
ticks <- time.Now()
res, err := Watch(f, []PRRef{open.Ref}, "unkin-agent", ticks, nil, func(PRRef, error) { warned++ })
if err != nil {
t.Fatalf("Watch: %v", err)
}
if warned != 1 {
t.Errorf("warnings = %d, want 1", warned)
}
if res.Reason != "PR merged" {
t.Errorf("reason = %q, want %q", res.Reason, "PR merged")
}
}
// The production failure: a watched repo was renamed mid-watch, so every poll
// 404'd (Gitea hides a repo the caller may not see rather than 403ing) and the
// loop warned past it forever while reporting nothing. A 404 on a tracked PR
// must end the watch with an error naming that PR.
func TestWatchAbortsOnMidRunNotFound(t *testing.T) {
const sha = "deadbeefdeadbeef"
var polls atomic.Int32
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
if polls.Add(1) > 1 { // repo renamed/made private after the baseline
w.WriteHeader(http.StatusNotFound)
_, _ = fmt.Fprint(w, `{"message":"Not Found"}`)
return
}
_, _ = fmt.Fprintf(w, `{"number":7,"state":"open","merged":false,"mergeable":true,"head":{"sha":%q}}`, sha)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/commits/"+sha+"/status", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `{"state":"success"}`)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/7/comments", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `[]`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
ref := PRRef{Owner: "unkin", Repo: "repo", Number: 7}
tk := time.NewTicker(5 * time.Millisecond)
defer tk.Stop()
var warned atomic.Int32
done := make(chan error, 1)
go func() {
_, err := Watch(c, []PRRef{ref}, "unkin-agent", tk.C, nil,
func(PRRef, error) { warned.Add(1) })
done <- err
}()
select {
case err := <-done:
if err == nil {
t.Fatal("Watch should abort on a mid-run 404, not keep polling")
}
if !IsNotFound(err) {
t.Errorf("Watch error = %v, want a 404", err)
}
if !strings.Contains(err.Error(), ref.String()) {
t.Errorf("Watch error = %v, want it to name %s", err, ref.String())
}
if n := warned.Load(); n != 0 {
t.Errorf("404 was logged as a warning %d time(s); it must abort", n)
}
case <-time.After(3 * time.Second):
t.Fatal("Watch hung: a vanished repo was warned past instead of aborting")
}
}
// A 404 from a sub-resource is not proof the PR is gone: an ingress can serve
// one during a Gitea rolling restart. Only the PR lookup itself is authoritative,
// so a comments 404 must warn and keep polling like any other transient failure,
// and still catch the merge that lands afterwards.
func TestWatchSurvivesCommentsNotFound(t *testing.T) {
const sha = "0badc0de0badc0de"
var polls atomic.Int32
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
if polls.Add(1) >= 4 {
_, _ = fmt.Fprintf(w, `{"number":7,"state":"closed","merged":true,"mergeable":true,"head":{"sha":%q}}`, sha)
return
}
_, _ = fmt.Fprintf(w, `{"number":7,"state":"open","merged":false,"mergeable":true,"head":{"sha":%q}}`, sha)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/commits/"+sha+"/status", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `{"state":"success"}`)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/7/comments", func(w http.ResponseWriter, r *http.Request) {
if n := polls.Load(); n == 2 || n == 3 { // proxy blip across two polls
w.WriteHeader(http.StatusNotFound)
_, _ = fmt.Fprint(w, `{"message":"Not Found"}`)
return
}
_, _ = fmt.Fprint(w, `[]`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
ref := PRRef{Owner: "unkin", Repo: "repo", Number: 7}
tk := time.NewTicker(5 * time.Millisecond)
defer tk.Stop()
var warned atomic.Int32
type outcome struct {
res WatchResult
err error
}
done := make(chan outcome, 1)
go func() {
res, err := Watch(c, []PRRef{ref}, "unkin-agent", tk.C, nil,
func(PRRef, error) { warned.Add(1) })
done <- outcome{res, err}
}()
select {
case o := <-done:
if o.err != nil {
t.Fatalf("Watch: %v (a comments 404 must not be terminal)", o.err)
}
if o.res.Reason != "PR merged" {
t.Errorf("reason = %q, want %q", o.res.Reason, "PR merged")
}
if n := warned.Load(); n != 2 {
t.Errorf("warnings = %d, want 2", n)
}
case <-time.After(3 * time.Second):
t.Fatal("Watch hung: a comments 404 must warn and keep polling")
}
}
// A comments 404 costs a poll from the same budget as any other failure: it must
// not be free, and a permanently 404ing sub-resource must still end the watch.
func TestWatchCommentsNotFoundCountsTowardCap(t *testing.T) {
const sha = "1badc0de1badc0de"
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprintf(w, `{"number":7,"state":"open","merged":false,"mergeable":true,"head":{"sha":%q}}`, sha)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/commits/"+sha+"/status", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `{"state":"success"}`)
})
var comments atomic.Int32
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/7/comments", func(w http.ResponseWriter, r *http.Request) {
if comments.Add(1) > 1 { // healthy at baseline, gone from the first poll on
w.WriteHeader(http.StatusNotFound)
_, _ = fmt.Fprint(w, `{"message":"Not Found"}`)
return
}
_, _ = fmt.Fprint(w, `[]`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
ref := PRRef{Owner: "unkin", Repo: "repo", Number: 7}
tk := time.NewTicker(time.Millisecond)
defer tk.Stop()
var warned atomic.Int32
done := make(chan error, 1)
go func() {
_, err := Watch(c, []PRRef{ref}, "unkin-agent", tk.C, nil,
func(PRRef, error) { warned.Add(1) })
done <- err
}()
select {
case err := <-done:
if err == nil {
t.Fatal("Watch should give up once the comments 404 stops being transient")
}
if !strings.Contains(err.Error(), "consecutive failures") {
t.Errorf("Watch error = %v, want it to report the failure cap", err)
}
if n := warned.Load(); n != MaxPollFailures {
t.Errorf("warnings = %d, want %d", n, MaxPollFailures)
}
case <-time.After(3 * time.Second):
t.Fatal("Watch hung: a permanently 404ing comments endpoint must hit the cap")
}
}
// The PR lookup is the call whose 404 means the PR is gone, so it aborts on the
// very first occurrence rather than spending the failure budget.
func TestWatchAbortsOnFirstPRLookupNotFound(t *testing.T) {
const sha = "2badc0de2badc0de"
var polls atomic.Int32
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
if polls.Add(1) > 1 {
w.WriteHeader(http.StatusNotFound)
_, _ = fmt.Fprint(w, `{"message":"Not Found"}`)
return
}
_, _ = fmt.Fprintf(w, `{"number":7,"state":"open","merged":false,"mergeable":true,"head":{"sha":%q}}`, sha)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/commits/"+sha+"/status", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `{"state":"success"}`)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/7/comments", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `[]`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
ref := PRRef{Owner: "unkin", Repo: "repo", Number: 7}
tk := time.NewTicker(5 * time.Millisecond)
defer tk.Stop()
done := make(chan error, 1)
go func() {
_, err := Watch(c, []PRRef{ref}, "unkin-agent", tk.C, nil,
func(PRRef, error) { t.Errorf("a PR-lookup 404 must abort, not warn") })
done <- err
}()
select {
case err := <-done:
if !IsNotFound(err) {
t.Fatalf("Watch error = %v, want a 404", err)
}
if n := polls.Load(); n != 2 {
t.Errorf("PR fetches = %d, want 2 (baseline + the 404 that aborts)", n)
}
case <-time.After(3 * time.Second):
t.Fatal("Watch hung: a vanished PR must abort")
}
}
// A 5xx blip must not kill a long watch: it warns, keeps polling, and still
// catches the merge that lands afterwards.
func TestWatchSurvivesTransientServerError(t *testing.T) {
const sha = "feedfacefeedface"
var polls atomic.Int32
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) {
switch n := polls.Add(1); {
case n == 2 || n == 3: // gateway blip across two polls
w.WriteHeader(http.StatusBadGateway)
_, _ = fmt.Fprint(w, `bad gateway`)
case n >= 4:
_, _ = fmt.Fprintf(w, `{"number":7,"state":"closed","merged":true,"mergeable":true,"head":{"sha":%q}}`, sha)
default:
_, _ = fmt.Fprintf(w, `{"number":7,"state":"open","merged":false,"mergeable":true,"head":{"sha":%q}}`, sha)
}
})
mux.HandleFunc("/api/v1/repos/unkin/repo/commits/"+sha+"/status", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `{"state":"success"}`)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/issues/7/comments", func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `[]`)
})
srv := httptest.NewServer(mux)
defer srv.Close()
c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
ref := PRRef{Owner: "unkin", Repo: "repo", Number: 7}
tk := time.NewTicker(5 * time.Millisecond)
defer tk.Stop()
var warned atomic.Int32
type outcome struct {
res WatchResult
err error
}
done := make(chan outcome, 1)
go func() {
res, err := Watch(c, []PRRef{ref}, "unkin-agent", tk.C, nil,
func(PRRef, error) { warned.Add(1) })
done <- outcome{res, err}
}()
select {
case o := <-done:
if o.err != nil {
t.Fatalf("Watch: %v", o.err)
}
if o.res.Reason != "PR merged" {
t.Errorf("reason = %q, want %q", o.res.Reason, "PR merged")
}
if n := warned.Load(); n != 2 {
t.Errorf("warnings = %d, want 2", n)
}
case <-time.After(3 * time.Second):
t.Fatal("Watch hung: a transient 5xx must not stop the watch")
}
}
// pollFailures scripts a fetcher whose polls fail with a 502 at the given call
// indexes (0 is the baseline); the final call returns merged.
func pollFailures(calls int, failAt map[int]bool) *fakeFetcher {
open, merged := base(), base()
merged.State = "closed"
merged.Merged = true
f := &fakeFetcher{states: make([]PRState, calls), errs: make([]error, calls)}
for i := range calls {
f.states[i] = open
if failAt[i] {
f.errs[i] = &APIError{Method: "GET", Path: "/p", StatusCode: 502, Body: "bad gateway"}
}
}
f.states[calls-1] = merged
return f
}
// A permanently wedged endpoint (5xx forever) must eventually give up instead of
// warning on every tick for the life of the process.
func TestWatchAbortsAfterConsecutiveFailures(t *testing.T) {
failAt := map[int]bool{}
for i := 1; i <= MaxPollFailures; i++ {
failAt[i] = true
}
f := pollFailures(MaxPollFailures+1, failAt)
warned := 0
ticks := make(chan time.Time, MaxPollFailures)
for range MaxPollFailures {
ticks <- time.Now()
}
close(ticks)
_, err := Watch(f, []PRRef{base().Ref}, "unkin-agent", ticks, nil, func(PRRef, error) { warned++ })
if err == nil {
t.Fatal("Watch should give up once the failures stop being transient")
}
if !strings.Contains(err.Error(), "consecutive failures") {
t.Errorf("Watch error = %v, want it to report the failure cap", err)
}
if warned != MaxPollFailures {
t.Errorf("warnings = %d, want %d", warned, MaxPollFailures)
}
if f.calls != MaxPollFailures+1 {
t.Errorf("fetch calls = %d, want %d", f.calls, MaxPollFailures+1)
}
}
// The cap counts consecutive failures only: a single successful poll clears it,
// so an intermittent endpoint is watched indefinitely and the merge is caught.
func TestWatchFailureCountResetsOnSuccess(t *testing.T) {
const runs = MaxPollFailures - 1
failAt := map[int]bool{}
for i := 1; i <= runs; i++ { // first run of failures
failAt[i] = true
}
for i := runs + 2; i <= 2*runs+1; i++ { // second run, after one good poll
failAt[i] = true
}
f := pollFailures(2*runs+3, failAt)
ticks := make(chan time.Time, 2*runs+2)
for range 2*runs + 2 {
ticks <- time.Now()
}
close(ticks)
res, err := Watch(f, []PRRef{base().Ref}, "unkin-agent", ticks, nil, func(PRRef, error) {})
if err != nil {
t.Fatalf("Watch: %v (a successful poll must reset the failure count)", err)
}
if res.Reason != "PR merged" {
t.Errorf("reason = %q, want %q", res.Reason, "PR merged")
}
}
// Anonymous watching of a public repo must poll on without a credential in
// sight: no token, no mint, no exit until something actually changes.
func TestWatchAnonymousKeepsPolling(t *testing.T) {
open := base()
f := &fakeFetcher{states: []PRState{open}}
ticks := make(chan time.Time, 2)
ticks <- time.Now()
ticks <- time.Now()
close(ticks)
res, err := Watch(f, []PRRef{open.Ref}, "unkin-agent", ticks, nil,
func(_ PRRef, e error) { t.Errorf("unexpected poll error: %v", e) })
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "" {
t.Errorf("reason = %q, want no change reported", res.Reason)
}
if f.calls != 3 {
t.Errorf("fetch calls = %d, want 3 (baseline + two polls)", f.calls)
}
}