15 Commits

Author SHA1 Message Date
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
benvin d9645ec5e4 Merge pull request 'Add agentvault seed-oauth for oauth2-proxy credentials' (#9) from benvin/agentvault-seed-oauth into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #9
2026-08-30 15:41:13 +10:00
unkin-agent 155392a809 Add agentvault seed-oauth for oauth2-proxy credentials
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Seeding an oauth2-proxy secret by hand means an agent shell-plumbing a
client secret and a cookie secret, which the classifier blocks. seed-oauth
does it in one self-contained invocation: it reads the KV path, fills in
only the keys that are missing, preserves everything else and prints key
names and the new version, never a value.

- Add SeedOAuth in internal/agent: read-modify-write of the client_id,
  client_secret and cookie_secret keys with per-key created/kept/rotated
  actions and a no-op when nothing changed.
- Generate secrets from 32 crypto/rand bytes; cookie_secret is base64url so
  it decodes to exactly the 32 bytes oauth2-proxy requires.
- Add ReadKVOptional (missing secret = empty) and WriteKVAny (non-string
  fields survive a round trip) to the KV-v2 client.
- Wire the seed-oauth subcommand and document it in README and AGENTS.md.
- Cover fresh create, patch-preserves-client_secret, other-key
  preservation, --rotate, idempotence, denial errors and secret leakage.
2026-08-30 15:31:25 +10:00
benvin 47118215b4 Merge pull request 'Make agentpr's Vault gitea creds path selectable' (#8) from benvin/agentpr-creds-path into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #8
2026-08-30 09:36:22 +10:00
unkin-agent 26cd05e961 Make the Vault gitea creds path selectable
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
agentpr always read gitea/creds/unkin-agent from a bare const, so a service
like repospawner could not run it as its own Gitea identity.

- Replace the GiteaCredsPath const with a function: GITEA_CREDS_PATH when set,
  otherwise gitea/creds/<AGENT_LOGIN>. Unset env still resolves to
  gitea/creds/unkin-agent, so existing callers are unchanged.
- Thread the creds path through fetchGiteaToken/readGiteaCreds instead of
  reading a package-level const, and report it in the error messages.
- Make agentpr's help text login-agnostic and document both variables.
2026-08-30 00:50:35 +10:00
24 changed files with 3505 additions and 86 deletions
+58 -11
View File
@@ -3,8 +3,10 @@
## Project Overview
This repo ships several Gitea-automation CLIs in one RPM (`agent-tools`). They
act as the `unkin-agent` user by minting a scoped Gitea token from Vault, so
actions are attributed to the agent rather than to whoever runs the tool.
act as an agent user (`unkin-agent` by default) by minting a scoped Gitea token
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:
@@ -17,7 +19,7 @@ actions are attributed to the agent rather than to whoever runs the tool.
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
@@ -29,10 +31,11 @@ parsing, watch-state comparison, git worktree helpers).
cmd/agentpr/main.go # agentpr CLI (pr create / pr comment / whoami)
cmd/watchpr/main.go # watchpr CLI (poll + meaningful-change exit)
cmd/agentws/main.go # agentws CLI (new / list / rm / clean / token / credential)
cmd/agentvault/main.go # agentvault CLI (seed-outpost)
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 gitea/creds/unkin-agent
vault.go # AppRole login + read the gitea creds path
gitea.go # Gitea REST client (PR create/get, comments, status, whoami)
parse.go # owner/repo#N and owner/repo parsing
watch.go # PRState snapshot + MeaningfulChange comparison
@@ -40,6 +43,7 @@ internal/agent/ # shared plumbing:
vaultkv.go # AppRole-authenticated Vault client + KV-v2 read/write
authentik.go # Authentik REST client (outpost search, token view_key)
seedoutpost.go # seed-outpost flow (Authentik token -> Vault KV)
seedoauth.go # seed-oauth flow (oauth2-proxy credential set in Vault KV)
go.mod # module git.unkin.net/unkin/agent-tools
Makefile # build / test / lint / completions / rpm / version-bump
packaging/nfpm.yaml # nfpm spec (envsubst-templated) for the RPM (all binaries)
@@ -57,7 +61,9 @@ All tools call `agent.GiteaToken()`, which (once per process):
1. AppRole login: `POST $VAULT_ADDR/v1/auth/approle/login` with `role_id` only
(no `secret_id`) → `client_token`.
2. `GET $VAULT_ADDR/v1/gitea/creds/unkin-agent` with `X-Vault-Token``.data.token`.
2. `GET $VAULT_ADDR/v1/<creds path>` with `X-Vault-Token``.data.token`, where
the creds path is `GITEA_CREDS_PATH` if set, else `gitea/creds/$AGENT_LOGIN`
(so unset env still reads `gitea/creds/unkin-agent`).
Config via env (all have defaults):
@@ -66,7 +72,8 @@ Config via env (all have defaults):
| `VAULT_ADDR` | `https://vault.service.consul:8200` | Vault/OpenBao address |
| `AGENT_APPROLE_ROLE_ID` | built-in default | AppRole role_id (overridable) |
| `GITEA_URL` | `https://git.unkin.net` | Gitea base URL |
| `AGENT_LOGIN` | `unkin-agent` | login whose comments watchpr ignores; agentws git identity |
| `AGENT_LOGIN` | `unkin-agent` | agent identity: selects `gitea/creds/<login>`; login whose comments watchpr ignores; agentws git identity |
| `GITEA_CREDS_PATH` | `gitea/creds/$AGENT_LOGIN` | Vault path minting the Gitea token (wins over `AGENT_LOGIN`) |
| `AGENTWS_SRC_ROOT` | `~/src/prodenv` | agentws source-of-truth checkout root |
| `AGENTWS_ROOT` | `~/.cache/agentws` | agentws worktree root |
| `AGENTWS_OWNER` | `unkin` | Gitea org that owns agentws-managed repos |
@@ -77,7 +84,7 @@ Config via env (all have defaults):
Gitea tokens are ~1h ephemeral, so `agentws` never bakes one into a remote URL
or config. `agentws token` prints a fresh token; `agentws credential get`
implements the git credential protocol (reads the key=value request on stdin,
and for the configured Gitea host only emits `username=unkin-agent` +
and for the configured Gitea host only emits `username=$AGENT_LOGIN` +
`password=<fresh token>`). `agentws new` wires this per worktree — it enables
`extensions.worktreeConfig` on the repo once, then writes `user.name`,
`user.email` and `credential.helper = !<agentws> credential` to the
@@ -143,12 +150,52 @@ printed. Errors are wrapped per stage (login / read denied / outpost missing /
view_key / write denied) via the `ErrVaultDenied`, `ErrVaultNotFound` and
`ErrOutpostNotFound` sentinels.
## agentvault seed-oauth
`agentvault seed-oauth --path <kv/path> --client-id <id>` makes a KV-v2 path
hold a complete oauth2-proxy credential set, in-process:
1. AppRole login (shared `approleLogin`), then a KV-v2 read via
`ReadKVOptional` — a 404 or a deleted version means "empty", not an error,
so the first seed of a path works.
2. Desired keys are computed over the existing map: `client_id` from the flag
(`kept`/`created`/`updated`), `client_secret` and `cookie_secret` generated
from 32 `crypto/rand` bytes only when absent or when `--rotate` is set
(`kept`/`created`/`rotated`). `cookie_secret` is base64url so it decodes to
exactly the 32 bytes oauth2-proxy demands; `client_secret` is standard
base64.
3. Any other key on the path is carried through unchanged (`preserved`), which
is why the write goes through `WriteKVAny` rather than `WriteKV`.
4. The write is skipped entirely when nothing changed; the command then prints
`version: unchanged`.
Only key names, per-key actions and the new KV version are printed. Errors are
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 on a
401/403 and replays the request. If the fresh token is rejected too, `watchpr`
exits non-zero rather than polling blind.
- `watchpr` polls anonymously when no token can be minted (public repos work
fine); only a real 401/403 reaches for Vault.
- 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.
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.
- CI "combined status" comes from `/commits/{sha}/status`; an empty head SHA
yields an empty state without an API call.
+98 -10
View File
@@ -1,11 +1,12 @@
# agent-tools
Small Gitea-automation CLIs, shipped together in one RPM (`agent-tools`). They
act as the **`unkin-agent`** user by minting a scoped Gitea token from Vault, so
automated PRs, comments and pushes are attributed to the agent — not to whoever
happens to run the command.
act as an agent user (**`unkin-agent`** by default) by minting a scoped Gitea
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 `unkin-agent`.
- **`agentpr`** — create pull requests and post PR comments 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
@@ -16,8 +17,9 @@ happens to run the command.
## How it gets a token
On first use each tool performs a Vault AppRole login (`role_id` only, no
`secret_id`), then reads `gitea/creds/unkin-agent` to obtain a short-lived Gitea
token, cached in-process for the run.
`secret_id`), then reads `gitea/creds/$AGENT_LOGIN` — or `GITEA_CREDS_PATH` when
set — to obtain a short-lived Gitea token, cached in-process for the run. With
neither variable set that is `gitea/creds/unkin-agent`, as before.
Everything is configured by environment variables, all with defaults:
@@ -26,7 +28,8 @@ Everything is configured by environment variables, all with defaults:
| `VAULT_ADDR` | `https://vault.service.consul:8200` | Vault/OpenBao address |
| `AGENT_APPROLE_ROLE_ID` | built-in default | AppRole role_id (overridable) |
| `GITEA_URL` | `https://git.unkin.net` | Gitea base URL |
| `AGENT_LOGIN` | `unkin-agent` | login whose comments `watchpr` ignores |
| `AGENT_LOGIN` | `unkin-agent` | agent identity: selects `gitea/creds/<login>`, and the login whose comments `watchpr` ignores |
| `GITEA_CREDS_PATH` | `gitea/creds/$AGENT_LOGIN` | Vault path minting the Gitea token (wins over `AGENT_LOGIN`) |
| `AGENTWS_SRC_ROOT` | `~/src/prodenv` | source-of-truth checkout root (`agentws`) |
| `AGENTWS_ROOT` | `~/.cache/agentws` | worktree root (`agentws`) |
| `AGENTWS_OWNER` | `unkin` | Gitea org that owns the repos (`agentws`) |
@@ -35,7 +38,7 @@ Everything is configured by environment variables, all with defaults:
## agentpr
```bash
# Verify identity (should print: unkin-agent)
# Verify identity (prints the agent login, unkin-agent by default)
agentpr whoami
# Open a PR
@@ -67,6 +70,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
@@ -98,13 +104,59 @@ agentws list
agentws rm benvin/my-change
agentws rm ~/.cache/agentws/argocd-apps__benvin-my-change --delete-branch
# Classify every managed worktree; dry run unless --yes is given
agentws prune
agentws prune --yes
agentws prune --yes --keep-branches
# Remove every managed worktree and prune each source repo
agentws clean
# Print a fresh unkin-agent Gitea token
# Print a fresh Gitea token for the agent login
agentws token
```
### prune
`agentws prune` decides, per worktree, whether its work is safely upstream:
| Signal (first match wins) | Verdict |
|---|---|
| uncommitted 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 |
| 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.
`--keep-branches` removes worktrees only, and its verdicts print as `remove`.
### Auth / credential-helper design
Gitea tokens minted from Vault are short-lived (~1h), so `agentws` never
@@ -113,7 +165,7 @@ persists one in a remote URL or in git config. Instead it wires itself as an
- `agentws token` prints a fresh token to stdout (handy for scripts).
- `agentws credential get` speaks the git credential protocol on stdin and, for
the configured Gitea host only, emits `username=unkin-agent` +
the configured Gitea host only, emits `username=$AGENT_LOGIN` +
`password=<fresh token>`.
`agentws new` sets this up per worktree without touching the shared checkout: it
@@ -159,6 +211,42 @@ Errors name the failing stage: AppRole login, KV read denied (policy not
applied), outpost not found (terraform not applied), `view_key` failure, or KV
write denied.
### seed-oauth
Make a Vault KV-v2 path hold a complete oauth2-proxy credential set. It is a
read-modify-write: `client_id` is set from the flag, `client_secret` and
`cookie_secret` are generated (32 bytes from `crypto/rand`) only when missing,
every other key on the path is written back untouched, and nothing is written
at all when the secret is already correct. `cookie_secret` is base64url so it
decodes to exactly the 32 bytes oauth2-proxy requires.
```bash
agentvault seed-oauth \
--path kubernetes/namespace/repospawner/default/oauth-credentials \
--client-id 4f1c…
```
```
path: kv/kubernetes/namespace/repospawner/default/oauth-credentials
keys: client_id, client_secret, cookie_secret
client_id: created
client_secret: kept
cookie_secret: created
version: 4
```
That is the common case: the provider's `client_secret` already lives on the
path, so only the missing keys are added. A run with nothing to do prints
`version: unchanged` and issues no write.
Flags: `--path` and `--client-id` are required; `--kv-mount` (default `kv`) and
`--rotate` override the rest. `--rotate` regenerates both secrets — only use it
when the IdP provider's secret is being rotated alongside, since a rotated
`client_secret` no longer matches the provider.
Errors name the failing stage: AppRole login, KV read denied, or KV write
denied. Only key names, actions and the KV version are printed.
## Build & package
```bash
+7 -7
View File
@@ -1,7 +1,7 @@
// Command agentpr manages Gitea pull requests and comments as the unkin-agent
// user. It obtains a scoped Gitea token from Vault (AppRole login, then reads
// gitea/creds/unkin-agent) so actions are attributed to the agent rather than
// to whoever runs the tool.
// Command agentpr manages Gitea pull requests and comments as an agent user. It
// obtains a scoped Gitea token from Vault (AppRole login, then reads
// gitea/creds/<AGENT_LOGIN>, or GITEA_CREDS_PATH when set) so actions are
// attributed to that agent rather than to whoever runs the tool.
//
// agentpr pr create --repo owner/repo --base main --head feature --title T --body B
// agentpr pr comment --repo owner/repo --pr 12 --body "..."
@@ -33,8 +33,8 @@ func main() {
func newRootCmd() *cobra.Command {
root := &cobra.Command{
Use: "agentpr",
Short: "Manage Gitea PRs and comments as the unkin-agent user.",
Long: "agentpr manages Gitea pull requests and comments as unkin-agent, using a\nGitea token minted from Vault (AppRole login + gitea/creds/unkin-agent).",
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.",
Version: version,
SilenceUsage: true,
}
@@ -146,7 +146,7 @@ func newPRCommentCmd() *cobra.Command {
func newWhoamiCmd() *cobra.Command {
return &cobra.Command{
Use: "whoami",
Short: "Print the authenticated Gitea login (should be unkin-agent)",
Short: "Print the authenticated Gitea login (the identity PRs are opened as)",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
c, err := client()
+45 -1
View File
@@ -3,11 +3,13 @@
// with the same Vault AppRole as agentpr (role_id only, no secret_id).
//
// agentvault seed-outpost --outpost <name> --dest-path <kv/path>
// agentvault seed-oauth --path <kv/path> --client-id <id>
package main
import (
"fmt"
"os"
"strings"
"git.unkin.net/unkin/agent-tools/internal/agent"
@@ -33,7 +35,7 @@ func newRootCmd() *cobra.Command {
SilenceUsage: true,
}
root.SetVersionTemplate("{{.Version}}\n")
root.AddCommand(newSeedOutpostCmd(), newVersionCmd())
root.AddCommand(newSeedOutpostCmd(), newSeedOAuthCmd(), newVersionCmd())
return root
}
@@ -73,6 +75,48 @@ func newSeedOutpostCmd() *cobra.Command {
return cmd
}
func newSeedOAuthCmd() *cobra.Command {
opts := agent.SeedOAuthOptions{}
cmd := &cobra.Command{
Use: "seed-oauth",
Short: "Seed an oauth2-proxy credential set into Vault KV",
Long: "Make a Vault KV-v2 path hold a complete oauth2-proxy credential set: the\n" +
"given client_id, plus a client_secret and a 32-byte cookie_secret that are\n" +
"generated only when missing (or with --rotate). Existing keys are preserved\n" +
"and nothing is written when the secret is already correct. Secret values are\n" +
"never printed or logged.",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
opts.VaultAddr = agent.VaultAddr()
opts.RoleID = agent.RoleID()
res, err := agent.SeedOAuth(opts)
if err != nil {
return err
}
out := cmd.OutOrStdout()
_, _ = fmt.Fprintf(out, "path: %s/%s\n", res.KVMount, res.Path)
_, _ = fmt.Fprintf(out, "keys: %s\n", strings.Join(res.KeyNames(), ", "))
for _, k := range res.Keys {
_, _ = fmt.Fprintf(out, " %-14s %s\n", k.Name+":", k.Action)
}
if res.Changed {
_, _ = fmt.Fprintf(out, "version: %d\n", res.Version)
} else {
_, _ = fmt.Fprintf(out, "version: unchanged\n")
}
return nil
},
}
f := cmd.Flags()
f.StringVar(&opts.Path, "path", "", "KV-v2 path holding the credentials, e.g. kubernetes/namespace/repospawner/default/oauth-credentials (required)")
f.StringVar(&opts.ClientID, "client-id", "", "OIDC client id to store (required)")
f.StringVar(&opts.KVMount, "kv-mount", agent.DefaultKVMount, "KV-v2 mount holding the path")
f.BoolVar(&opts.Rotate, "rotate", false, "Regenerate client_secret and cookie_secret even when they already exist")
_ = cmd.MarkFlagRequired("path")
_ = cmd.MarkFlagRequired("client-id")
return cmd
}
func newVersionCmd() *cobra.Command {
return &cobra.Command{
Use: "version",
+84
View File
@@ -2,6 +2,7 @@ package main
import (
"bytes"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
@@ -78,6 +79,89 @@ func TestSeedOutpostOutputHasNoSecrets(t *testing.T) {
}
}
const oauthPath = "kubernetes/namespace/repospawner/default/oauth-credentials"
// fakeOAuthVault serves approle login plus a KV-v2 path that already holds a
// client_secret, and records what gets written back.
func fakeOAuthVault(t *testing.T, existing map[string]string) (vaultURL string, written *map[string]string) {
t.Helper()
writes := map[string]string{}
mux := http.NewServeMux()
mux.HandleFunc("/v1/auth/approle/login", func(w http.ResponseWriter, r *http.Request) {
_, _ = io.WriteString(w, `{"auth":{"client_token":"s.vaulttoken"}}`)
})
mux.HandleFunc("/v1/kv/data/"+oauthPath, func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodPost {
var body struct {
Data map[string]string `json:"data"`
}
_ = json.NewDecoder(r.Body).Decode(&body)
for k, v := range body.Data {
writes[k] = v
}
_, _ = io.WriteString(w, `{"data":{"version":4}}`)
return
}
payload, _ := json.Marshal(map[string]any{"data": map[string]any{"data": existing}})
_, _ = w.Write(payload)
})
srv := httptest.NewServer(mux)
t.Cleanup(srv.Close)
return srv.URL, &writes
}
// The command prints key names and the KV version only — never a value.
func TestSeedOAuthOutputHasNoSecrets(t *testing.T) {
const existingSecret = "existing-client-secret-value"
vaultURL, written := fakeOAuthVault(t, map[string]string{"client_secret": existingSecret})
t.Setenv("VAULT_ADDR", vaultURL)
t.Setenv("AGENT_APPROLE_ROLE_ID", "role-xyz")
var out bytes.Buffer
cmd := newRootCmd()
cmd.SetOut(&out)
cmd.SetErr(&out)
cmd.SetArgs([]string{"seed-oauth", "--path", oauthPath, "--client-id", "mediamark-client-id"})
if err := cmd.Execute(); err != nil {
t.Fatalf("Execute: %v", err)
}
got := out.String()
for _, want := range []string{"kv/" + oauthPath, "client_id, client_secret, cookie_secret", "client_secret: kept", "cookie_secret: created", "version: 4"} {
if !strings.Contains(got, want) {
t.Errorf("output missing %q:\n%s", want, got)
}
}
for key, value := range *written {
if key == "client_id" {
continue
}
if strings.Contains(got, value) {
t.Fatalf("output leaks the %s value:\n%s", key, got)
}
}
if strings.Contains(got, existingSecret) {
t.Fatalf("output leaks the existing client_secret:\n%s", got)
}
}
func TestSeedOAuthRequiresFlags(t *testing.T) {
for name, args := range map[string][]string{
"no path": {"seed-oauth", "--client-id", "mediamark-client-id"},
"no client-id": {"seed-oauth", "--path", oauthPath},
} {
t.Run(name, func(t *testing.T) {
cmd := newRootCmd()
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
cmd.SetArgs(args)
if err := cmd.Execute(); err == nil {
t.Fatal("Execute() = nil, want a missing-required-flag error")
}
})
}
}
func TestSeedOutpostRequiresFlags(t *testing.T) {
for name, args := range map[string][]string{
"no outpost": {"seed-outpost", "--dest-path", destPath},
+20 -4
View File
@@ -13,6 +13,7 @@
// agentws new <repo> [--branch benvin/<name>] [--from <base-branch>]
// agentws list
// agentws rm <path-or-branch> [--delete-branch]
// agentws prune [--yes] [--keep-branches]
// agentws clean
// agentws token
// agentws credential get # git credential-helper protocol on stdin
@@ -54,6 +55,7 @@ func newRootCmd() *cobra.Command {
newNewCmd(),
newListCmd(),
newRmCmd(),
newPruneCmd(),
newCleanCmd(),
newTokenCmd(),
newCredentialCmd(),
@@ -294,7 +296,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")
@@ -316,13 +319,16 @@ func resolveWorktree(target string) (managedWt, error) {
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 {
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 +340,16 @@ func removeWorktree(out io.Writer, wt managedWt, deleteBranch bool) error {
return agent.GitWorktreePrune(wt.srcDir)
}
// 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 +368,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
}
}
+295
View File
@@ -0,0 +1,295 @@
package main
import (
"errors"
"fmt"
"io"
"path/filepath"
"sort"
"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
}
func newPruneCmd() *cobra.Command {
var apply, keepBranches bool
cmd := &cobra.Command{
Use: "prune",
Short: "Classify managed worktrees and remove the ones whose work is safely upstream",
Long: "prune inspects every managed worktree, classifies it against git and its Gitea\npull request, and removes the ones whose work is provably upstream. It is a dry\nrun unless --yes is given.",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
return runPrune(cmd.OutOrStdout(), pruneClient(), apply, keepBranches)
},
}
f := cmd.Flags()
f.BoolVar(&apply, "yes", false, "Actually remove worktrees (default is a dry run)")
f.BoolVar(&keepBranches, "keep-branches", false, "Never delete a local branch, whatever the classification")
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 io.Writer, prs prLister, apply, keepBranches bool) error {
managed, err := managedWorktrees()
if err != nil {
return err
}
if len(managed) == 0 {
_, _ = fmt.Fprintln(out, "no managed worktrees")
return nil
}
byRepo := map[string][]managedWt{}
for _, w := range managed {
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)
var results []pruneResult
for _, srcDir := range srcDirs {
ctx, err := newRepoCtx(out, prs, srcDir)
if err != nil {
for _, w := range byRepo[srcDir] {
results = append(results, pruneResult{wt: w, verdict: verdictKeep, reason: "repo state unknown: " + 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: " + err.Error()}
}
results = append(results, res)
}
}
for _, r := range results {
_, _ = fmt.Fprintf(out, "%-44s %-34s %-14s %s\n", filepath.Base(r.wt.path), r.wt.branch, plannedVerdict(r, keepBranches), r.reason)
}
if !apply {
_, _ = fmt.Fprintln(out, "dry run: nothing removed (pass --yes to apply)")
return nil
}
var errs []error
for _, r := range results {
if r.verdict == verdictKeep {
continue
}
deleteBranch := r.verdict == verdictRemoveBranch && !keepBranches
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...)
}
// plannedVerdict is what will actually happen, so --keep-branches does not
// print a branch deletion it will not perform.
func plannedVerdict(r pruneResult, keepBranches bool) string {
if keepBranches && r.verdict == verdictRemoveBranch {
return verdictRemove
}
return r.verdict
}
// newRepoCtx refreshes a source repo and collects the signals prune classifies
// against. A failed fetch or an unreachable Gitea is 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) (repoCtx, error) {
ctx := repoCtx{srcDir: srcDir, prs: map[string]agent.PullRequest{}}
repo := filepath.Base(srcDir)
if err := agent.GitFetchPrune(srcDir, "origin", credentialHelperArgs()...); err != nil {
_, _ = 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: dirty and open-PR worktrees are kept,
// provably-upstream work loses its branch too, and anything unproven keeps its
// branch so no commits become 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}
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
}
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, fetch failed so %s is unverified", pr.Number, 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, fetch failed so %s is unverified", pr.Number, 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"
}
return res, nil
}
+687
View File
@@ -0,0 +1,687 @@
package main
import (
"bytes"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"testing"
"git.unkin.net/unkin/agent-tools/internal/agent"
)
// fixture is a bare origin plus a source checkout named "repo" and a worktree
// root, wired so managedWorktrees() finds the worktrees created here.
type fixture struct {
root string
bare string
srcDir string
wtRoot string
}
func git(t *testing.T, dir string, args ...string) string {
t.Helper()
cmd := exec.Command("git", args...)
cmd.Dir = dir
out, err := cmd.CombinedOutput()
if err != nil {
t.Fatalf("git %s (in %s): %v: %s", strings.Join(args, " "), dir, err, out)
}
return strings.TrimSpace(string(out))
}
func newFixture(t *testing.T) *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"),
}
if err := os.MkdirAll(filepath.Join(root, "src"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.MkdirAll(f.wtRoot, 0o755); err != nil {
t.Fatal(err)
}
git(t, root, "init", "--bare", "-b", "main", f.bare)
seed := filepath.Join(root, "seed")
git(t, root, "init", "-b", "main", 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", "main")
git(t, filepath.Join(root, "src"), "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
}
func identity(t *testing.T, dir string) {
t.Helper()
git(t, dir, "config", "user.email", "test@example.com")
git(t, dir, "config", "user.name", "Test")
}
func writeCommit(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)
}
git(t, dir, "add", ".")
git(t, dir, "commit", "-m", msg)
}
// addWorktree creates a managed worktree for branch and returns its path.
func (f *fixture) addWorktree(t *testing.T, branch string) string {
t.Helper()
path := filepath.Join(f.wtRoot, agent.WorktreeDirName("repo", branch))
git(t, f.srcDir, "worktree", "add", path, "-b", branch, "origin/main")
identity(t, path)
return path
}
// landUpstream commits content on origin's main, mimicking a squash merge: the
// same patch arrives upstream under a different SHA.
func (f *fixture) landUpstream(t *testing.T, name, content, msg string) {
t.Helper()
seed := filepath.Join(f.root, "seed")
git(t, seed, "pull", "--ff-only", "origin", "main")
writeCommit(t, seed, name, content, msg)
git(t, seed, "push", "origin", "main")
}
// fakeGitea serves the pulls listing for unkin/repo with the given PR bodies.
func fakeGitea(t *testing.T, prs ...map[string]any) *httptest.Server {
t.Helper()
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls", func(w http.ResponseWriter, r *http.Request) {
if r.URL.Query().Get("page") != "1" {
_, _ = w.Write([]byte("[]"))
return
}
body, err := json.Marshal(prs)
if err != nil {
t.Errorf("marshal PRs: %v", err)
}
_, _ = w.Write(body)
})
srv := httptest.NewServer(mux)
t.Cleanup(srv.Close)
return srv
}
func client(srv *httptest.Server) prLister {
return &agent.GiteaClient{BaseURL: srv.URL, HTTP: srv.Client()}
}
// mergedPR mimics Gitea after a merge: the branch is deleted, so head.ref
// becomes refs/pull/<n>/head and only head.label still names the branch.
func mergedPR(number int, branch string) map[string]any {
return map[string]any{
"number": number,
"state": "closed",
"merged": true,
"head": map[string]any{
"ref": "refs/pull/" + strconv.Itoa(number) + "/head",
"label": branch,
},
}
}
func withHeadSha(pr map[string]any, sha string) map[string]any {
pr["head"].(map[string]any)["sha"] = sha
return pr
}
func openPR(number int, branch string) map[string]any {
return map[string]any{
"number": number,
"state": "open",
"merged": false,
"head": map[string]any{"ref": branch, "label": branch},
}
}
func closedPR(number int, branch string) map[string]any {
return map[string]any{
"number": number,
"state": "closed",
"merged": false,
"head": map[string]any{"ref": branch, "label": branch},
}
}
func run(t *testing.T, prs prLister, apply, keepBranches bool) string {
t.Helper()
var out bytes.Buffer
if err := runPrune(&out, prs, apply, keepBranches); err != nil {
t.Fatalf("runPrune: %v\n%s", err, out.String())
}
return out.String()
}
func lineFor(t *testing.T, out, branch string) string {
t.Helper()
for _, line := range strings.Split(out, "\n") {
if strings.Contains(line, " "+branch+" ") {
return line
}
}
t.Fatalf("no line for branch %q in:\n%s", branch, out)
return ""
}
func assertVerdict(t *testing.T, out, branch, verdict, reason string) {
t.Helper()
line := lineFor(t, out, branch)
fields := strings.Fields(line)
if len(fields) < 3 || fields[2] != verdict {
t.Errorf("branch %s: verdict line %q, want verdict %q", branch, line, verdict)
}
if reason != "" && !strings.Contains(line, reason) {
t.Errorf("branch %s: line %q, want reason containing %q", branch, line, reason)
}
}
func exists(path string) bool {
_, err := os.Stat(path)
return err == nil
}
// A merged PR's branch is deleted on merge, so its head.ref reads
// refs/pull/<n>/head; classification must still see the merge (via head.label)
// and remove the branch, not fall through to "no PR".
func TestPruneMergedPRWithDeletedBranch(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/merged")
writeCommit(t, wt, "m.txt", "m\n", "work")
head := git(t, wt, "rev-parse", "HEAD")
srv := fakeGitea(t, withHeadSha(mergedPR(3, "benvin/merged"), head))
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/merged", verdictRemoveBranch, "PR merged #3")
if exists(wt) {
t.Errorf("worktree %s should have been removed", wt)
}
if agent.GitBranchExists(f.srcDir, "benvin/merged") {
t.Error("branch of a merged PR should be deleted")
}
}
// The same PR without head.label: matching falls back to head.ref, which no
// longer names the branch, so prune must not guess it is merged — the worktree
// goes but the branch stays.
func TestPruneMergedPRWithoutLabelKeepsBranch(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/merged")
writeCommit(t, wt, "m.txt", "m\n", "work")
pr := mergedPR(3, "benvin/merged")
pr["head"].(map[string]any)["label"] = ""
srv := fakeGitea(t, pr)
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/merged", verdictRemove, "no PR")
if !agent.GitBranchExists(f.srcDir, "benvin/merged") {
t.Error("branch must survive when the PR could not be matched")
}
}
func TestPruneContainedBranch(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/contained")
srv := fakeGitea(t)
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/contained", verdictRemoveBranch, "contained in origin/main")
if exists(wt) {
t.Error("contained worktree should have been removed")
}
if agent.GitBranchExists(f.srcDir, "benvin/contained") {
t.Error("contained branch should be deleted")
}
}
// Squash-merged work keeps a local SHA that is not upstream, so only the
// patch-equivalence check proves it landed.
func TestPruneCherryCleanBranch(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/squashed")
writeCommit(t, wt, "s.txt", "same\n", "add s")
f.landUpstream(t, "s.txt", "same\n", "squashed s")
srv := fakeGitea(t)
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/squashed", verdictRemoveBranch, "patch-equivalent commits in origin/main history")
if agent.GitBranchExists(f.srcDir, "benvin/squashed") {
t.Error("cherry-clean branch should be deleted")
}
}
// Uncommitted work outranks every other signal, including a branch that is
// otherwise fully contained upstream.
func TestPruneNeverTouchesDirtyWorktree(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/dirty")
if err := os.WriteFile(filepath.Join(wt, "wip.txt"), []byte("wip\n"), 0o644); err != nil {
t.Fatal(err)
}
srv := fakeGitea(t, mergedPR(4, "benvin/dirty"))
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/dirty", verdictKeep, "dirty")
if !exists(wt) {
t.Error("dirty worktree must not be removed")
}
if !agent.GitBranchExists(f.srcDir, "benvin/dirty") {
t.Error("dirty worktree's branch must survive")
}
}
// An open PR is kept even when its commits are already upstream.
func TestPruneKeepsOpenPR(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/open")
srv := fakeGitea(t, openPR(5, "benvin/open"))
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/open", verdictKeep, "PR open #5")
if !exists(wt) {
t.Error("worktree with an open PR must not be removed")
}
if !agent.GitBranchExists(f.srcDir, "benvin/open") {
t.Error("branch with an open PR must not be deleted")
}
}
// Closed-unmerged with the branch still on origin: the work is not lost, so the
// local branch goes too.
func TestPruneClosedPRWithBranchOnOrigin(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/closed")
writeCommit(t, wt, "c.txt", "c\n", "work")
git(t, wt, "push", "origin", "benvin/closed")
srv := fakeGitea(t, closedPR(6, "benvin/closed"))
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/closed", verdictRemoveBranch, "PR closed #6, HEAD contained in origin/benvin/closed")
if exists(wt) {
t.Error("worktree should have been removed")
}
if agent.GitBranchExists(f.srcDir, "benvin/closed") {
t.Error("branch should be deleted while origin still has it")
}
}
// Closed-unmerged with nothing on origin: the commits exist only here, so the
// branch is kept and only the worktree goes.
func TestPruneClosedPRWithBranchGone(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/orphan")
writeCommit(t, wt, "o.txt", "o\n", "work")
srv := fakeGitea(t, closedPR(7, "benvin/orphan"))
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/orphan", verdictRemove, "PR closed #7, branch gone")
if exists(wt) {
t.Error("worktree should have been removed")
}
if !agent.GitBranchExists(f.srcDir, "benvin/orphan") {
t.Error("branch must survive when origin does not have the commits")
}
}
func TestPruneNoPRKeepsBranch(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/unpushed")
writeCommit(t, wt, "u.txt", "u\n", "work")
srv := fakeGitea(t)
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/unpushed", verdictRemove, "no PR")
if exists(wt) {
t.Error("worktree should have been removed")
}
if !agent.GitBranchExists(f.srcDir, "benvin/unpushed") {
t.Error("branch with unproven work must survive")
}
}
// The default run reports and changes nothing.
func TestPruneDryRunChangesNothing(t *testing.T) {
f := newFixture(t)
contained := f.addWorktree(t, "benvin/contained")
merged := f.addWorktree(t, "benvin/merged")
writeCommit(t, merged, "m.txt", "m\n", "work")
head := git(t, merged, "rev-parse", "HEAD")
srv := fakeGitea(t, withHeadSha(mergedPR(8, "benvin/merged"), head))
out := run(t, client(srv), false, false)
if !strings.Contains(out, "dry run") {
t.Errorf("dry-run output should say so:\n%s", out)
}
assertVerdict(t, out, "benvin/contained", verdictRemoveBranch, "contained")
assertVerdict(t, out, "benvin/merged", verdictRemoveBranch, "PR merged #8")
if !exists(contained) || !exists(merged) {
t.Error("dry run must not remove worktrees")
}
if !agent.GitBranchExists(f.srcDir, "benvin/contained") || !agent.GitBranchExists(f.srcDir, "benvin/merged") {
t.Error("dry run must not delete branches")
}
}
// --keep-branches removes worktrees but leaves every branch alone, and the
// printed verdict says so.
func TestPruneKeepBranches(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/contained")
srv := fakeGitea(t)
out := run(t, client(srv), true, true)
assertVerdict(t, out, "benvin/contained", verdictRemove, "contained")
if strings.Contains(out, verdictRemoveBranch) {
t.Errorf("--keep-branches must not print a branch-deleting verdict:\n%s", out)
}
if exists(wt) {
t.Error("worktree should have been removed")
}
if !agent.GitBranchExists(f.srcDir, "benvin/contained") {
t.Error("--keep-branches must not delete the branch")
}
}
// With Gitea unreachable prune falls back to the git signals: provably-upstream
// work is still cleaned up, and anything unproven keeps its branch.
func TestPruneDegradesWhenGiteaUnreachable(t *testing.T) {
f := newFixture(t)
contained := f.addWorktree(t, "benvin/contained")
unproven := f.addWorktree(t, "benvin/unproven")
writeCommit(t, unproven, "u.txt", "u\n", "work")
dead := httptest.NewServer(http.NewServeMux())
c := &agent.GiteaClient{BaseURL: dead.URL, HTTP: dead.Client()}
dead.Close()
out := run(t, c, true, false)
if !strings.Contains(out, "git signals only") {
t.Errorf("output should note the Gitea failure:\n%s", out)
}
assertVerdict(t, out, "benvin/contained", verdictRemoveBranch, "contained")
assertVerdict(t, out, "benvin/unproven", verdictRemove, "PR state unknown")
if exists(contained) || exists(unproven) {
t.Error("both worktrees should have been removed")
}
if agent.GitBranchExists(f.srcDir, "benvin/contained") {
t.Error("contained branch is safe to delete without Gitea")
}
if !agent.GitBranchExists(f.srcDir, "benvin/unproven") {
t.Error("unproven branch must survive an unreachable Gitea")
}
}
func TestPruneNoWorktrees(t *testing.T) {
newFixture(t)
srv := fakeGitea(t)
if out := run(t, client(srv), true, false); !strings.Contains(out, "no managed worktrees") {
t.Errorf("output = %q", out)
}
}
// Commits made after the PR merged exist nowhere else, so a merged PR alone
// must not authorise deleting the branch.
func TestPruneMergedPRWithCommitsAfterMerge(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/ahead")
writeCommit(t, wt, "a.txt", "a\n", "merged work")
merged := git(t, wt, "rev-parse", "HEAD")
writeCommit(t, wt, "b.txt", "b\n", "work after the merge")
srv := fakeGitea(t, withHeadSha(mergedPR(9, "benvin/ahead"), merged))
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/ahead", verdictRemove, "PR merged #9")
if exists(wt) {
t.Error("worktree should have been removed")
}
if !agent.GitBranchExists(f.srcDir, "benvin/ahead") {
t.Error("branch with commits beyond the merged head must survive")
}
}
// HEAD proven contained in the merged head still loses its branch.
func TestPruneMergedPRContainedInMergedHead(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/landed")
writeCommit(t, wt, "a.txt", "a\n", "work")
head := git(t, wt, "rev-parse", "HEAD")
srv := fakeGitea(t, withHeadSha(mergedPR(10, "benvin/landed"), head))
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/landed", verdictRemoveBranch, "HEAD contained in the merged head")
if exists(wt) {
t.Error("worktree should have been removed")
}
if agent.GitBranchExists(f.srcDir, "benvin/landed") {
t.Error("branch contained in the merged head should be deleted")
}
}
// A surviving remote branch only covers what was pushed to it; later local
// commits keep the branch.
func TestPruneClosedPRWithCommitsBeyondOrigin(t *testing.T) {
f := newFixture(t)
wt := f.addWorktree(t, "benvin/beyond")
writeCommit(t, wt, "c.txt", "c\n", "pushed work")
git(t, wt, "push", "origin", "benvin/beyond")
writeCommit(t, wt, "d.txt", "d\n", "local only")
srv := fakeGitea(t, closedPR(11, "benvin/beyond"))
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/beyond", verdictRemove, "local commits not on origin/benvin/beyond")
if exists(wt) {
t.Error("worktree should have been removed")
}
if !agent.GitBranchExists(f.srcDir, "benvin/beyond") {
t.Error("branch with commits beyond origin must survive")
}
}
// truncatedLister stands in for a repo with more PRs than the listing cap.
type truncatedLister struct{ prs []agent.PullRequest }
func (l truncatedLister) ListPRs(string, string) ([]agent.PullRequest, error) {
return l.prs, fmt.Errorf("unkin/repo: %w after 1000 pull requests", agent.ErrPRListTruncated)
}
// A truncated listing still classifies the PRs it saw, but a branch missing
// from it reads as unknown rather than as having no PR.
func TestPruneWarnsOnTruncatedPRListing(t *testing.T) {
f := newFixture(t)
listed := f.addWorktree(t, "benvin/listed")
writeCommit(t, listed, "a.txt", "a\n", "work")
head := git(t, listed, "rev-parse", "HEAD")
unlisted := f.addWorktree(t, "benvin/unlisted")
writeCommit(t, unlisted, "b.txt", "b\n", "work")
var pr agent.PullRequest
pr.Number, pr.State, pr.Merged = 12, "closed", true
pr.Head.Label, pr.Head.Sha = "benvin/listed", head
out := run(t, truncatedLister{prs: []agent.PullRequest{pr}}, true, false)
if !strings.Contains(out, "truncated") || !strings.Contains(out, "older PRs unseen") {
t.Errorf("output should warn about the truncated listing:\n%s", out)
}
assertVerdict(t, out, "benvin/listed", verdictRemoveBranch, "PR merged #12")
assertVerdict(t, out, "benvin/unlisted", verdictRemove, "PR state unknown")
if agent.GitBranchExists(f.srcDir, "benvin/listed") {
t.Error("branch of a merged PR seen in the listing should be deleted")
}
if !agent.GitBranchExists(f.srcDir, "benvin/unlisted") {
t.Error("branch missing from a truncated listing must survive")
}
}
// breakRemote points origin at a path that does not exist, so every fetch fails.
func (f *fixture) breakRemote(t *testing.T) {
t.Helper()
git(t, f.srcDir, "remote", "set-url", "origin", filepath.Join(f.root, "missing.git"))
}
// applyUnreachable applies the plan against a repo whose remote is unreachable.
// The post-removal refresh fetch fails, so runPrune must report an error; the
// classification and the removals it authorised happen regardless.
func applyUnreachable(t *testing.T, prs prLister) string {
t.Helper()
var out bytes.Buffer
err := runPrune(&out, prs, true, false)
if err == nil {
t.Fatalf("expected the refresh fetch to fail against a missing remote:\n%s", out.String())
}
return out.String()
}
// staleFixture builds a repo where origin/<branch> covers HEAD for a closed and
// a merged PR. Deleting the branches on origin makes those tracking refs stale:
// a pruning fetch would drop them, so they only prove anything while a fetch
// this run confirms they are still there.
func staleFixture(t *testing.T, deleteUpstream bool) (*fixture, *httptest.Server, string, string) {
t.Helper()
f := newFixture(t)
closed := f.addWorktree(t, "benvin/stale-closed")
writeCommit(t, closed, "c.txt", "c\n", "work")
git(t, closed, "push", "origin", "benvin/stale-closed")
merged := f.addWorktree(t, "benvin/stale-merged")
writeCommit(t, merged, "m.txt", "m\n", "work")
git(t, merged, "push", "origin", "benvin/stale-merged")
if deleteUpstream {
git(t, f.bare, "update-ref", "-d", "refs/heads/benvin/stale-closed")
git(t, f.bare, "update-ref", "-d", "refs/heads/benvin/stale-merged")
}
// The merged head is an older commit, so only origin/<branch> covers HEAD.
base := git(t, f.srcDir, "rev-parse", "origin/main")
srv := fakeGitea(t,
closedPR(20, "benvin/stale-closed"),
withHeadSha(mergedPR(21, "benvin/stale-merged"), base),
)
return f, srv, closed, merged
}
// A tracking ref this run's fetch could not confirm is not evidence: the branch
// may already be gone upstream, and the next successful --prune deletes the ref.
// Both worktrees go, both branches stay.
func TestPruneFailedFetchDistrustsStaleRemoteBranch(t *testing.T) {
f, srv, closed, merged := staleFixture(t, true)
f.breakRemote(t)
out := run(t, client(srv), false, false)
if !strings.Contains(out, "remote state unverified") {
t.Errorf("output should report the failed fetch:\n%s", out)
}
assertVerdict(t, out, "benvin/stale-closed", verdictRemove, "PR closed #20, fetch failed so origin/benvin/stale-closed is unverified")
assertVerdict(t, out, "benvin/stale-merged", verdictRemove, "PR merged #21, fetch failed so origin/benvin/stale-merged is unverified")
for _, b := range []string{"benvin/stale-closed", "benvin/stale-merged"} {
if !agent.GitRemoteBranchExists(f.srcDir, "origin", b) {
t.Fatalf("fixture: origin/%s should still be present as a stale ref", b)
}
}
applyUnreachable(t, client(srv))
if exists(closed) || exists(merged) {
t.Error("both worktrees should have been removed")
}
for _, b := range []string{"benvin/stale-closed", "benvin/stale-merged"} {
if !agent.GitBranchExists(f.srcDir, b) {
t.Errorf("branch %s must survive an unverified origin", b)
}
}
}
// The control for the case above: with the fetch working and the branches still
// on origin, the same shape still loses both branches.
func TestPruneSuccessfulFetchTrustsRemoteBranch(t *testing.T) {
f, srv, closed, merged := staleFixture(t, false)
out := run(t, client(srv), true, false)
assertVerdict(t, out, "benvin/stale-closed", verdictRemoveBranch, "PR closed #20, HEAD contained in origin/benvin/stale-closed")
assertVerdict(t, out, "benvin/stale-merged", verdictRemoveBranch, "PR merged #21, HEAD contained in origin/benvin/stale-merged")
if exists(closed) || exists(merged) {
t.Error("both worktrees should have been removed")
}
for _, b := range []string{"benvin/stale-closed", "benvin/stale-merged"} {
if agent.GitBranchExists(f.srcDir, b) {
t.Errorf("branch %s should be deleted while origin still has it", b)
}
}
}
// Proofs that read only local objects and the merged head SHA from the API do
// not depend on the fetch, so a failed fetch must not suppress them.
func TestPruneFailedFetchKeepsFetchIndependentProofs(t *testing.T) {
f := newFixture(t)
contained := f.addWorktree(t, "benvin/contained")
landed := f.addWorktree(t, "benvin/landed")
writeCommit(t, landed, "a.txt", "a\n", "work")
head := git(t, landed, "rev-parse", "HEAD")
f.breakRemote(t)
srv := fakeGitea(t, withHeadSha(mergedPR(22, "benvin/landed"), head))
out := applyUnreachable(t, client(srv))
assertVerdict(t, out, "benvin/contained", verdictRemoveBranch, "contained in origin/main")
assertVerdict(t, out, "benvin/landed", verdictRemoveBranch, "PR merged #22, HEAD contained in the merged head")
if exists(contained) || exists(landed) {
t.Error("both worktrees should have been removed")
}
for _, b := range []string{"benvin/contained", "benvin/landed"} {
if agent.GitBranchExists(f.srcDir, b) {
t.Errorf("branch %s is proven without the fetch and should be deleted", b)
}
}
}
// Managed repos are not all under AGENTWS_OWNER, so the Gitea path comes from
// origin's URL; a non-Gitea remote falls back to the configured owner.
func TestRepoPathFollowsOrigin(t *testing.T) {
t.Setenv("AGENTWS_OWNER", "unkin")
dir := t.TempDir()
git(t, dir, "init", "-b", "main", ".")
git(t, dir, "remote", "add", "origin", "https://git.unkin.net/unkinben/dotfiles.git")
if got := repoPath(dir, "dotfiles"); got != "unkinben/dotfiles" {
t.Errorf("repoPath = %q, want unkinben/dotfiles", got)
}
git(t, dir, "remote", "set-url", "origin", filepath.Join(dir, "origin.git"))
if got := repoPath(dir, "dotfiles"); got != "unkin/dotfiles" {
t.Errorf("repoPath for a local remote = %q, want unkin/dotfiles", got)
}
}
+21 -9
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.
@@ -131,6 +137,12 @@ func runWatch(c *agent.GiteaClient, refs []agent.PRRef, interval time.Duration,
res, err := agent.Watch(c, refs, login, ticker.C, onBaseline, onError)
if err != nil {
if agent.IsAuthError(err) {
return fmt.Errorf("gitea authentication failed after re-minting the token, watch aborted: %w", err)
}
if agent.IsPRGone(err) {
return fmt.Errorf("PR no longer visible (repo deleted, renamed, or made private), watch aborted: %w", err)
}
return err
}
report(res.Ref.String(), res.Reason, res.State, jsonMode)
+77
View File
@@ -2,6 +2,9 @@ package main
import (
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
)
@@ -28,3 +31,77 @@ 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)
}
}
}
+320 -6
View File
@@ -2,14 +2,18 @@ package agent
import (
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
)
// fakeVault serves the AppRole login and gitea creds endpoints.
func fakeVault(t *testing.T, wantRoleID, giteaToken string) *httptest.Server {
// fakeVault serves the AppRole login and the gitea creds secret at credsPath
// only, so a read of any other path 404s.
func fakeVault(t *testing.T, wantRoleID, credsPath, giteaToken string) *httptest.Server {
t.Helper()
mux := http.NewServeMux()
mux.HandleFunc("/v1/auth/approle/login", func(w http.ResponseWriter, r *http.Request) {
@@ -26,7 +30,7 @@ func fakeVault(t *testing.T, wantRoleID, giteaToken string) *httptest.Server {
}
_, _ = io.WriteString(w, `{"auth":{"client_token":"s.vaulttoken"}}`)
})
mux.HandleFunc("/v1/"+GiteaCredsPath, func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/v1/"+credsPath, func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("X-Vault-Token"); got != "s.vaulttoken" {
t.Errorf("X-Vault-Token = %q, want s.vaulttoken", got)
}
@@ -36,10 +40,10 @@ func fakeVault(t *testing.T, wantRoleID, giteaToken string) *httptest.Server {
}
func TestFetchGiteaToken(t *testing.T) {
srv := fakeVault(t, "role-xyz", "gitea-abc")
srv := fakeVault(t, "role-xyz", "gitea/creds/unkin-agent", "gitea-abc")
defer srv.Close()
tok, err := fetchGiteaToken(srv.URL, "role-xyz")
tok, err := fetchGiteaToken(srv.URL, "role-xyz", "gitea/creds/unkin-agent")
if err != nil {
t.Fatalf("fetchGiteaToken: %v", err)
}
@@ -57,7 +61,7 @@ func TestFetchGiteaTokenLoginError(t *testing.T) {
srv := httptest.NewServer(mux)
defer srv.Close()
if _, err := fetchGiteaToken(srv.URL, "role-xyz"); err == nil {
if _, err := fetchGiteaToken(srv.URL, "role-xyz", "gitea/creds/unkin-agent"); err == nil {
t.Fatal("expected error on 403 login")
}
}
@@ -216,6 +220,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) {
@@ -230,3 +328,219 @@ 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)
}
}
+67
View File
@@ -2,6 +2,7 @@ package agent
import (
"bytes"
"errors"
"fmt"
"os"
"os/exec"
@@ -67,6 +68,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 +92,64 @@ 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
}
// 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
}
// 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 {
+128
View File
@@ -185,6 +185,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()
+101 -8
View File
@@ -24,40 +24,75 @@ 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: the token is expired or
// unauthorised, which retrying the same request cannot fix.
func IsAuthError(err error) bool {
var apiErr *APIError
return errors.As(err, &apiErr) &&
(apiErr.StatusCode == http.StatusUnauthorized || apiErr.StatusCode == http.StatusForbidden)
}
// 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 token was rejected, re-mints it once and
// replays the request with the fresh token.
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
}
err := c.attempt(method, path, payload, out)
if !IsAuthError(err) || c.Refresh == nil {
return err
}
token, refreshErr := c.Refresh()
if refreshErr != nil {
return fmt.Errorf("%w; re-minting token: %v", err, refreshErr)
}
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 +137,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"`
+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)
}
}
}
+183
View File
@@ -0,0 +1,183 @@
package agent
import (
"crypto/rand"
"encoding/base64"
"errors"
"fmt"
"sort"
)
const (
// OAuthClientIDKey, OAuthClientSecretKey and OAuthCookieSecretKey are the
// KV fields oauth2-proxy deployments read their credentials from.
OAuthClientIDKey = "client_id"
OAuthClientSecretKey = "client_secret"
OAuthCookieSecretKey = "cookie_secret"
// oauthSecretBytes is the raw length of generated secrets. oauth2-proxy
// rejects a cookie secret that does not decode to exactly 32 bytes.
oauthSecretBytes = 32
)
// Per-key outcomes reported by SeedOAuth.
const (
ActionCreated = "created"
ActionKept = "kept"
ActionRotated = "rotated"
ActionUpdated = "updated"
ActionPreserved = "preserved"
)
// SeedOAuthOptions configures SeedOAuth. The CLI supplies the defaults.
type SeedOAuthOptions struct {
VaultAddr string
RoleID string
KVMount string
Path string
ClientID string
Rotate bool
}
// SeedOAuthKey names a key present in the secret and what happened to it.
type SeedOAuthKey struct {
Name string
Action string
}
// SeedOAuthResult is the non-secret summary of a seed run. Version is zero when
// nothing changed and no write was issued.
type SeedOAuthResult struct {
KVMount string
Path string
Keys []SeedOAuthKey
Version int
Changed bool
}
// KeyNames returns the key names present in the secret after the run.
func (r SeedOAuthResult) KeyNames() []string {
names := make([]string, 0, len(r.Keys))
for _, k := range r.Keys {
names = append(names, k.Name)
}
return names
}
// SeedOAuth makes a Vault KV-v2 path hold a complete oauth2-proxy credential
// set: client_id from the caller, plus a client_secret and cookie_secret that
// are generated only when absent (or when Rotate is set). It is a
// read-modify-write, so keys it does not own survive untouched, and it writes
// nothing when the secret is already correct. Secret material never leaves this
// function: results and errors carry only key names.
func SeedOAuth(o SeedOAuthOptions) (SeedOAuthResult, error) {
var res SeedOAuthResult
if o.Path == "" {
return res, errors.New("a KV-v2 path is required")
}
if o.ClientID == "" {
return res, errors.New("a client id is required")
}
vc, err := NewVaultClient(o.VaultAddr, o.RoleID)
if err != nil {
return res, fmt.Errorf("vault approle login failed against %s (check VAULT_ADDR and AGENT_APPROLE_ROLE_ID): %w", o.VaultAddr, err)
}
existing, err := vc.ReadKVOptional(o.KVMount, o.Path)
if err != nil {
if errors.Is(err, ErrVaultDenied) {
return res, fmt.Errorf("reading %s/%s denied: the agent AppRole policy does not grant read on this path (apply the terraform-vault policy change): %w", o.KVMount, o.Path, err)
}
return res, fmt.Errorf("reading %s/%s: %w", o.KVMount, o.Path, err)
}
data := make(map[string]any, len(existing)+3)
for k, v := range existing {
data[k] = v
}
var (
keys []SeedOAuthKey
changed bool
)
current, _ := existing[OAuthClientIDKey].(string)
switch current {
case o.ClientID:
keys = append(keys, SeedOAuthKey{OAuthClientIDKey, ActionKept})
case "":
keys = append(keys, SeedOAuthKey{OAuthClientIDKey, ActionCreated})
changed = true
default:
keys = append(keys, SeedOAuthKey{OAuthClientIDKey, ActionUpdated})
changed = true
}
data[OAuthClientIDKey] = o.ClientID
for _, gen := range []struct {
name string
enc *base64.Encoding
}{
// oauth2-proxy accepts a standard-base64 client secret, but the cookie
// secret goes into a cookie and must be URL-safe.
{OAuthClientSecretKey, base64.StdEncoding},
{OAuthCookieSecretKey, base64.RawURLEncoding},
} {
current, _ := existing[gen.name].(string)
if current != "" && !o.Rotate {
keys = append(keys, SeedOAuthKey{gen.name, ActionKept})
continue
}
value, err := randomSecret(gen.enc)
if err != nil {
return res, fmt.Errorf("generating %s: %w", gen.name, err)
}
action := ActionCreated
if current != "" {
action = ActionRotated
}
data[gen.name] = value
keys = append(keys, SeedOAuthKey{gen.name, action})
changed = true
}
var others []string
for k := range existing {
switch k {
case OAuthClientIDKey, OAuthClientSecretKey, OAuthCookieSecretKey:
default:
others = append(others, k)
}
}
sort.Strings(others)
for _, k := range others {
keys = append(keys, SeedOAuthKey{k, ActionPreserved})
}
res = SeedOAuthResult{KVMount: o.KVMount, Path: o.Path, Keys: keys}
if !changed {
return res, nil
}
version, err := vc.WriteKVAny(o.KVMount, o.Path, data)
if err != nil {
if errors.Is(err, ErrVaultDenied) {
return SeedOAuthResult{}, fmt.Errorf("writing %s/%s denied: the agent AppRole policy does not grant create/update on this path (apply the terraform-vault policy change): %w", o.KVMount, o.Path, err)
}
return SeedOAuthResult{}, fmt.Errorf("writing %s/%s: %w", o.KVMount, o.Path, err)
}
res.Version = version
res.Changed = true
return res, nil
}
// randomSecret returns oauthSecretBytes of crypto/rand entropy in the given
// base64 encoding.
func randomSecret(enc *base64.Encoding) (string, error) {
buf := make([]byte, oauthSecretBytes)
if _, err := rand.Read(buf); err != nil {
return "", err
}
return enc.EncodeToString(buf), nil
}
+421
View File
@@ -0,0 +1,421 @@
package agent
import (
"encoding/base64"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"testing"
)
const (
oauthPath = "kubernetes/namespace/repospawner/default/oauth-credentials"
oauthClientID = "mediamark-client-id"
existingClientSec = "existing-client-secret-from-authentik"
existingCookieSec = "existing-cookie-secret-value-abcdefghij"
oauthExtraKeyValue = "extra-key-secret-value"
oauthVaultClientTok = "s.vaulttoken"
)
// oauthVaultStub is a KV-v2 stand-in that actually stores what is written, so
// read-modify-write behaviour can be asserted end to end.
type oauthVaultStub struct {
data map[string]any
exists bool
version int
readStatus int
writeStatus int
writes []map[string]any
}
func newOAuthVaultStub() *oauthVaultStub {
return &oauthVaultStub{readStatus: http.StatusOK, writeStatus: http.StatusOK}
}
// seed makes the path exist with the given fields at version 1.
func (v *oauthVaultStub) seed(data map[string]any) *oauthVaultStub {
v.data = data
v.exists = true
v.version = 1
return v
}
func (v *oauthVaultStub) server(t *testing.T) *httptest.Server {
t.Helper()
mux := http.NewServeMux()
mux.HandleFunc("/v1/auth/approle/login", func(w http.ResponseWriter, r *http.Request) {
var body map[string]string
_ = json.NewDecoder(r.Body).Decode(&body)
if _, ok := body["secret_id"]; ok {
t.Errorf("secret_id must not be sent")
}
_, _ = io.WriteString(w, `{"auth":{"client_token":"`+oauthVaultClientTok+`"}}`)
})
mux.HandleFunc("/v1/kv/data/"+oauthPath, func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("X-Vault-Token"); got != oauthVaultClientTok {
t.Errorf("X-Vault-Token = %q, want %q", got, oauthVaultClientTok)
}
switch r.Method {
case http.MethodGet:
if v.readStatus != http.StatusOK {
w.WriteHeader(v.readStatus)
_, _ = io.WriteString(w, `{"errors":["permission denied"]}`)
return
}
if !v.exists {
w.WriteHeader(http.StatusNotFound)
_, _ = io.WriteString(w, `{"errors":[]}`)
return
}
payload, _ := json.Marshal(map[string]any{
"data": map[string]any{"data": v.data, "metadata": map[string]any{"version": v.version}},
})
_, _ = w.Write(payload)
case http.MethodPost:
if v.writeStatus != http.StatusOK {
w.WriteHeader(v.writeStatus)
_, _ = io.WriteString(w, `{"errors":["permission denied"]}`)
return
}
var body struct {
Data map[string]any `json:"data"`
}
_ = json.NewDecoder(r.Body).Decode(&body)
v.writes = append(v.writes, body.Data)
v.data = body.Data
v.exists = true
v.version++
_, _ = io.WriteString(w, `{"data":{"version":`+strconv.Itoa(v.version)+`}}`)
default:
t.Errorf("unexpected method %s", r.Method)
}
})
srv := httptest.NewServer(mux)
t.Cleanup(srv.Close)
return srv
}
func oauthOpts(vaultURL string) SeedOAuthOptions {
return SeedOAuthOptions{
VaultAddr: vaultURL,
RoleID: "role-xyz",
KVMount: DefaultKVMount,
Path: oauthPath,
ClientID: oauthClientID,
}
}
// actions flattens a result into key -> action for order-independent asserts.
func actions(res SeedOAuthResult) map[string]string {
m := make(map[string]string, len(res.Keys))
for _, k := range res.Keys {
m[k.Name] = k.Action
}
return m
}
func stringField(t *testing.T, data map[string]any, key string) string {
t.Helper()
s, ok := data[key].(string)
if !ok {
t.Fatalf("written %s = %v, want a string", key, data[key])
}
return s
}
// assertDecodesTo32 fails unless the value is base64 of exactly 32 bytes, which
// is what oauth2-proxy requires of a cookie secret.
func assertDecodesTo32(t *testing.T, enc *base64.Encoding, value, name string) {
t.Helper()
raw, err := enc.DecodeString(value)
if err != nil {
t.Fatalf("%s is not valid base64: %v", name, err)
}
if len(raw) != oauthSecretBytes {
t.Errorf("%s decodes to %d bytes, want %d", name, len(raw), oauthSecretBytes)
}
}
func TestSeedOAuthFreshCreate(t *testing.T) {
v := newOAuthVaultStub()
res, err := SeedOAuth(oauthOpts(v.server(t).URL))
if err != nil {
t.Fatalf("SeedOAuth: %v", err)
}
if !res.Changed || res.Version != 1 {
t.Errorf("Changed=%v Version=%d, want a first write at version 1", res.Changed, res.Version)
}
for key, want := range map[string]string{
OAuthClientIDKey: ActionCreated,
OAuthClientSecretKey: ActionCreated,
OAuthCookieSecretKey: ActionCreated,
} {
if got := actions(res)[key]; got != want {
t.Errorf("%s action = %q, want %q", key, got, want)
}
}
if len(v.writes) != 1 {
t.Fatalf("%d writes, want exactly 1", len(v.writes))
}
w := v.writes[0]
if got := stringField(t, w, OAuthClientIDKey); got != oauthClientID {
t.Errorf("written client_id = %q, want %q", got, oauthClientID)
}
assertDecodesTo32(t, base64.StdEncoding, stringField(t, w, OAuthClientSecretKey), OAuthClientSecretKey)
assertDecodesTo32(t, base64.RawURLEncoding, stringField(t, w, OAuthCookieSecretKey), OAuthCookieSecretKey)
}
// The mediamark case: a client_secret already issued by Authentik must survive
// while the missing keys are filled in.
func TestSeedOAuthPreservesExistingClientSecret(t *testing.T) {
v := newOAuthVaultStub().seed(map[string]any{OAuthClientSecretKey: existingClientSec})
res, err := SeedOAuth(oauthOpts(v.server(t).URL))
if err != nil {
t.Fatalf("SeedOAuth: %v", err)
}
got := actions(res)
for key, want := range map[string]string{
OAuthClientIDKey: ActionCreated,
OAuthClientSecretKey: ActionKept,
OAuthCookieSecretKey: ActionCreated,
} {
if got[key] != want {
t.Errorf("%s action = %q, want %q", key, got[key], want)
}
}
if len(v.writes) != 1 {
t.Fatalf("%d writes, want exactly 1", len(v.writes))
}
if s := stringField(t, v.writes[0], OAuthClientSecretKey); s != existingClientSec {
t.Errorf("client_secret was replaced, want the existing value kept")
}
}
func TestSeedOAuthPreservesOtherKeys(t *testing.T) {
v := newOAuthVaultStub().seed(map[string]any{
OAuthClientIDKey: oauthClientID,
OAuthClientSecretKey: existingClientSec,
"redirect_url": "https://mediamark.unkin.net/oauth2/callback",
"extra": oauthExtraKeyValue,
})
res, err := SeedOAuth(oauthOpts(v.server(t).URL))
if err != nil {
t.Fatalf("SeedOAuth: %v", err)
}
got := actions(res)
for _, key := range []string{"redirect_url", "extra"} {
if got[key] != ActionPreserved {
t.Errorf("%s action = %q, want %q", key, got[key], ActionPreserved)
}
}
if len(v.writes) != 1 {
t.Fatalf("%d writes, want exactly 1", len(v.writes))
}
w := v.writes[0]
if stringField(t, w, "extra") != oauthExtraKeyValue {
t.Errorf("extra key was not written back unchanged")
}
if stringField(t, w, "redirect_url") != "https://mediamark.unkin.net/oauth2/callback" {
t.Errorf("redirect_url was not written back unchanged")
}
}
func TestSeedOAuthRotateRegenerates(t *testing.T) {
v := newOAuthVaultStub().seed(map[string]any{
OAuthClientIDKey: oauthClientID,
OAuthClientSecretKey: existingClientSec,
OAuthCookieSecretKey: existingCookieSec,
})
o := oauthOpts(v.server(t).URL)
o.Rotate = true
res, err := SeedOAuth(o)
if err != nil {
t.Fatalf("SeedOAuth: %v", err)
}
got := actions(res)
for key, want := range map[string]string{
OAuthClientIDKey: ActionKept,
OAuthClientSecretKey: ActionRotated,
OAuthCookieSecretKey: ActionRotated,
} {
if got[key] != want {
t.Errorf("%s action = %q, want %q", key, got[key], want)
}
}
if len(v.writes) != 1 {
t.Fatalf("%d writes, want exactly 1", len(v.writes))
}
w := v.writes[0]
if stringField(t, w, OAuthClientSecretKey) == existingClientSec {
t.Errorf("client_secret unchanged under --rotate")
}
if stringField(t, w, OAuthCookieSecretKey) == existingCookieSec {
t.Errorf("cookie_secret unchanged under --rotate")
}
assertDecodesTo32(t, base64.RawURLEncoding, stringField(t, w, OAuthCookieSecretKey), OAuthCookieSecretKey)
}
// A complete, correct secret must not produce a new KV version.
func TestSeedOAuthIdempotentWritesNothing(t *testing.T) {
v := newOAuthVaultStub()
url := v.server(t).URL
if _, err := SeedOAuth(oauthOpts(url)); err != nil {
t.Fatalf("first SeedOAuth: %v", err)
}
res, err := SeedOAuth(oauthOpts(url))
if err != nil {
t.Fatalf("second SeedOAuth: %v", err)
}
if res.Changed || res.Version != 0 {
t.Errorf("Changed=%v Version=%d, want an unchanged result", res.Changed, res.Version)
}
if len(v.writes) != 1 {
t.Errorf("%d writes, want the second run to write nothing", len(v.writes))
}
for _, k := range res.Keys {
if k.Action != ActionKept {
t.Errorf("%s action = %q, want %q", k.Name, k.Action, ActionKept)
}
}
}
func TestSeedOAuthClientIDUpdated(t *testing.T) {
v := newOAuthVaultStub().seed(map[string]any{
OAuthClientIDKey: "stale-client-id",
OAuthClientSecretKey: existingClientSec,
OAuthCookieSecretKey: existingCookieSec,
})
res, err := SeedOAuth(oauthOpts(v.server(t).URL))
if err != nil {
t.Fatalf("SeedOAuth: %v", err)
}
if got := actions(res)[OAuthClientIDKey]; got != ActionUpdated {
t.Errorf("client_id action = %q, want %q", got, ActionUpdated)
}
if len(v.writes) != 1 || stringField(t, v.writes[0], OAuthClientIDKey) != oauthClientID {
t.Errorf("writes = %v, want the new client_id written", v.writes)
}
}
func TestSeedOAuthLoginFailure(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/v1/auth/approle/login", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
_, _ = io.WriteString(w, `{"errors":["invalid role ID"]}`)
})
vs := httptest.NewServer(mux)
defer vs.Close()
_, err := SeedOAuth(oauthOpts(vs.URL))
if err == nil {
t.Fatal("SeedOAuth() = nil, want an approle login error")
}
if !strings.Contains(err.Error(), "approle login failed") {
t.Errorf("error = %v, want it to name the approle login", err)
}
}
func TestSeedOAuthReadDenied(t *testing.T) {
v := newOAuthVaultStub()
v.readStatus = http.StatusForbidden
_, err := SeedOAuth(oauthOpts(v.server(t).URL))
if err == nil {
t.Fatal("SeedOAuth() = nil, want a KV read error")
}
msg := err.Error()
if !strings.Contains(msg, oauthPath) || !strings.Contains(msg, "policy") {
t.Errorf("error = %v, want it to name the path and point at the policy", err)
}
if len(v.writes) != 0 {
t.Errorf("wrote %v, want no write when the read is denied", v.writes)
}
}
func TestSeedOAuthWriteDenied(t *testing.T) {
v := newOAuthVaultStub()
v.writeStatus = http.StatusForbidden
_, err := SeedOAuth(oauthOpts(v.server(t).URL))
if err == nil {
t.Fatal("SeedOAuth() = nil, want a KV write error")
}
msg := err.Error()
if !strings.Contains(msg, oauthPath) || !strings.Contains(msg, "create/update") {
t.Errorf("error = %v, want it to name the path and the missing capability", err)
}
}
// A missing path is normal (first seed), not a not-found error.
func TestSeedOAuthMissingPathIsNotAnError(t *testing.T) {
v := newOAuthVaultStub()
if _, err := SeedOAuth(oauthOpts(v.server(t).URL)); err != nil {
t.Fatalf("SeedOAuth on a missing path: %v", err)
}
}
func TestSeedOAuthRequiresPathAndClientID(t *testing.T) {
v := newOAuthVaultStub()
url := v.server(t).URL
for name, mutate := range map[string]func(*SeedOAuthOptions){
"no path": func(o *SeedOAuthOptions) { o.Path = "" },
"no client id": func(o *SeedOAuthOptions) { o.ClientID = "" },
} {
t.Run(name, func(t *testing.T) {
o := oauthOpts(url)
mutate(&o)
if _, err := SeedOAuth(o); err == nil {
t.Fatal("SeedOAuth() = nil, want a required-input error")
}
})
}
}
// No failure path may leak stored or generated secret material.
func TestSeedOAuthErrorsNeverLeakSecrets(t *testing.T) {
cases := map[string]func(*oauthVaultStub){
"read denied": func(v *oauthVaultStub) { v.readStatus = http.StatusForbidden },
"write denied": func(v *oauthVaultStub) { v.writeStatus = http.StatusForbidden },
"read error": func(v *oauthVaultStub) { v.readStatus = http.StatusInternalServerError },
"write error": func(v *oauthVaultStub) { v.writeStatus = http.StatusInternalServerError },
}
for name, mutate := range cases {
t.Run(name, func(t *testing.T) {
v := newOAuthVaultStub().seed(map[string]any{
OAuthClientSecretKey: existingClientSec,
OAuthCookieSecretKey: existingCookieSec,
"extra": oauthExtraKeyValue,
})
mutate(v)
_, err := SeedOAuth(oauthOpts(v.server(t).URL))
if err == nil {
t.Fatal("SeedOAuth() = nil, want an error")
}
for _, secret := range []string{existingClientSec, existingCookieSec, oauthExtraKeyValue} {
if strings.Contains(err.Error(), secret) {
t.Errorf("error %q leaks a secret", err)
}
}
})
}
}
// The successful result carries key names and a version, never values.
func TestSeedOAuthResultNeverCarriesSecrets(t *testing.T) {
v := newOAuthVaultStub().seed(map[string]any{OAuthClientSecretKey: existingClientSec})
res, err := SeedOAuth(oauthOpts(v.server(t).URL))
if err != nil {
t.Fatalf("SeedOAuth: %v", err)
}
rendered := strings.Join(append(res.KeyNames(), res.Path, res.KVMount), " ")
written := v.writes[0]
for _, key := range []string{OAuthClientSecretKey, OAuthCookieSecretKey} {
if value := stringField(t, written, key); strings.Contains(rendered, value) {
t.Errorf("result leaks the %s value", key)
}
}
}
+45 -17
View File
@@ -7,6 +7,7 @@ package agent
import (
"os"
"strings"
"sync"
)
@@ -16,12 +17,14 @@ const (
// DefaultRoleID is the agent AppRole role_id used when AGENT_APPROLE_ROLE_ID
// is unset. Login uses role_id only (no secret_id).
DefaultRoleID = "ababbcd3-9c77-5c6a-be2d-287fce9214a6"
// GiteaCredsPath is the Vault path that mints a scoped Gitea token.
GiteaCredsPath = "gitea/creds/unkin-agent"
// GiteaCredsPrefix is the Vault gitea secrets-engine creds prefix; the agent
// login is appended to it to form the path that mints a scoped Gitea token.
GiteaCredsPrefix = "gitea/creds/"
// DefaultGiteaURL is the Gitea base URL used when GITEA_URL is unset.
DefaultGiteaURL = "https://git.unkin.net"
// DefaultAgentLogin is the Gitea login of the agent whose own comments are
// ignored by watchpr. Overridable via AGENT_LOGIN.
// DefaultAgentLogin is the Gitea login the tools act as: it selects the Vault
// creds path, sets the agentws git identity and is the login whose own
// comments watchpr ignores. Overridable via AGENT_LOGIN.
DefaultAgentLogin = "unkin-agent"
// DefaultAuthentikURL is the Authentik base URL used when AUTHENTIK_URL is
// unset. identity.unkin.net has no DNS record; the k8s name is the real one.
@@ -53,8 +56,8 @@ func GiteaURL() string {
return DefaultGiteaURL
}
// AgentLogin returns the login whose comments watchpr ignores (env AGENT_LOGIN
// or the default).
// AgentLogin returns the Gitea login the tools act as (env AGENT_LOGIN or the
// default).
func AgentLogin() string {
if v := os.Getenv("AGENT_LOGIN"); v != "" {
return v
@@ -62,6 +65,17 @@ func AgentLogin() string {
return DefaultAgentLogin
}
// GiteaCredsPath returns the Vault path that mints a scoped Gitea token:
// GITEA_CREDS_PATH when set, otherwise gitea/creds/<AgentLogin>. So a service
// running as its own identity only has to set AGENT_LOGIN.
func GiteaCredsPath() string {
// Trimmed because callers join this onto ".../v1/".
if v := strings.Trim(strings.TrimSpace(os.Getenv("GITEA_CREDS_PATH")), "/"); v != "" {
return v
}
return GiteaCredsPrefix + AgentLogin()
}
// AuthentikURL returns the configured Authentik base URL (env AUTHENTIK_URL or
// the default).
func AuthentikURL() string {
@@ -72,27 +86,41 @@ 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() {
tokenValue, tokenErr = fetchGiteaToken(VaultAddr(), RoleID())
})
tokenMu.Lock()
defer tokenMu.Unlock()
if !tokenMinted {
tokenValue, tokenErr = fetchGiteaToken(VaultAddr(), RoleID(), GiteaCredsPath())
tokenMinted = true
}
return tokenValue, tokenErr
}
// fetchGiteaToken performs the AppRole login and reads the Gitea creds. It is
// separated from GiteaToken so tests can exercise it directly against an
// httptest server without touching the process-wide cache.
func fetchGiteaToken(vaultAddr, roleID string) (string, error) {
// 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
}
// fetchGiteaToken performs the AppRole login and reads the Gitea creds at
// credsPath. It is separated from GiteaToken so tests can exercise it directly
// against an httptest server without touching the process-wide cache.
func fetchGiteaToken(vaultAddr, roleID, credsPath string) (string, error) {
clientToken, err := approleLogin(vaultAddr, roleID)
if err != nil {
return "", err
}
return readGiteaCreds(vaultAddr, clientToken)
return readGiteaCreds(vaultAddr, clientToken, credsPath)
}
+155
View File
@@ -0,0 +1,155 @@
package agent
import (
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"strings"
"sync"
"testing"
)
func TestGiteaCredsPath(t *testing.T) {
tests := []struct {
name string
agentLogin string
credsPath string
want string
}{
{"no env keeps the historical path", "", "", "gitea/creds/unkin-agent"},
{"derived from AGENT_LOGIN", "repospawner", "", "gitea/creds/repospawner"},
{"GITEA_CREDS_PATH beats AGENT_LOGIN", "repospawner", "gitea/creds/someone-else", "gitea/creds/someone-else"},
{"GITEA_CREDS_PATH beats the default", "", "other-gitea/creds/bot", "other-gitea/creds/bot"},
{"override is trimmed for joining onto /v1/", "", " /gitea/creds/bot/ ", "gitea/creds/bot"},
{"blank override falls back to the login", "repospawner", " ", "gitea/creds/repospawner"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Setenv("AGENT_LOGIN", tt.agentLogin)
t.Setenv("GITEA_CREDS_PATH", tt.credsPath)
if got := GiteaCredsPath(); got != tt.want {
t.Errorf("GiteaCredsPath() = %q, want %q", got, tt.want)
}
})
}
}
// recordingVault serves the AppRole login plus any creds path under /v1/,
// recording which one was read so tests can assert the selected path.
type recordingVault struct {
mu sync.Mutex
credsPath string
}
func (v *recordingVault) path() string {
v.mu.Lock()
defer v.mu.Unlock()
return v.credsPath
}
// fakeEstate serves both Vault (/v1/...) and Gitea (/api/v1/...) from one
// server, so a test can drive the whole token-then-API flow.
func fakeEstate(t *testing.T, giteaToken, login string) (*httptest.Server, *recordingVault) {
t.Helper()
rec := &recordingVault{}
mux := http.NewServeMux()
mux.HandleFunc("/v1/auth/approle/login", func(w http.ResponseWriter, r *http.Request) {
_, _ = io.WriteString(w, `{"auth":{"client_token":"s.vaulttoken"}}`)
})
mux.HandleFunc("/v1/", func(w http.ResponseWriter, r *http.Request) {
rec.mu.Lock()
rec.credsPath = strings.TrimPrefix(r.URL.Path, "/v1/")
rec.mu.Unlock()
_, _ = io.WriteString(w, `{"data":{"token":"`+giteaToken+`"}}`)
})
mux.HandleFunc("/api/v1/user", func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("Authorization"); got != "token "+giteaToken {
t.Errorf("whoami auth header = %q, want token %s", got, giteaToken)
}
_, _ = io.WriteString(w, `{"login":"`+login+`","id":7}`)
})
mux.HandleFunc("/api/v1/repos/unkin/repo/pulls", func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("Authorization"); got != "token "+giteaToken {
t.Errorf("create PR auth header = %q, want token %s", got, giteaToken)
}
var body CreatePROptions
_ = json.NewDecoder(r.Body).Decode(&body)
_, _ = io.WriteString(w, `{"number":12,"state":"open","html_url":"`+r.Host+`/pulls/12"}`)
})
return httptest.NewServer(mux), rec
}
// A service that sets AGENT_LOGIN must read its own creds path and act as its
// own Gitea identity for both whoami and PR creation.
func TestWhoamiAndPRUseSelectedCredsPath(t *testing.T) {
srv, rec := fakeEstate(t, "gitea-repospawner", "repospawner")
defer srv.Close()
t.Setenv("AGENT_LOGIN", "repospawner")
t.Setenv("GITEA_CREDS_PATH", "")
t.Setenv("VAULT_ADDR", srv.URL)
t.Setenv("GITEA_URL", srv.URL)
tok, err := fetchGiteaToken(VaultAddr(), RoleID(), GiteaCredsPath())
if err != nil {
t.Fatalf("fetchGiteaToken: %v", err)
}
if got := rec.path(); got != "gitea/creds/repospawner" {
t.Errorf("vault read path = %q, want gitea/creds/repospawner", got)
}
if tok != "gitea-repospawner" {
t.Fatalf("token = %q, want gitea-repospawner", tok)
}
c := NewGiteaClient(tok)
u, err := c.Whoami()
if err != nil {
t.Fatalf("Whoami: %v", err)
}
if u.Login != "repospawner" {
t.Errorf("whoami login = %q, want repospawner", u.Login)
}
pr, err := c.CreatePR("unkin/repo", CreatePROptions{Base: "main", Head: "feature", Title: "T"})
if err != nil {
t.Fatalf("CreatePR: %v", err)
}
if pr.Number != 12 {
t.Errorf("PR number = %d, want 12", pr.Number)
}
}
// GITEA_CREDS_PATH must win even when AGENT_LOGIN names a different identity.
func TestCredsPathOverrideBeatsAgentLogin(t *testing.T) {
srv, rec := fakeEstate(t, "gitea-override", "someone-else")
defer srv.Close()
t.Setenv("AGENT_LOGIN", "repospawner")
t.Setenv("GITEA_CREDS_PATH", "gitea/creds/someone-else")
t.Setenv("VAULT_ADDR", srv.URL)
if _, err := fetchGiteaToken(VaultAddr(), RoleID(), GiteaCredsPath()); err != nil {
t.Fatalf("fetchGiteaToken: %v", err)
}
if got := rec.path(); got != "gitea/creds/someone-else" {
t.Errorf("vault read path = %q, want gitea/creds/someone-else", got)
}
}
// With no env set the tools must still read the exact path they always did.
func TestCredsPathDefaultIsBackwardCompatible(t *testing.T) {
srv, rec := fakeEstate(t, "gitea-abc", "unkin-agent")
defer srv.Close()
t.Setenv("AGENT_LOGIN", "")
t.Setenv("GITEA_CREDS_PATH", "")
t.Setenv("VAULT_ADDR", srv.URL)
if _, err := fetchGiteaToken(VaultAddr(), RoleID(), GiteaCredsPath()); err != nil {
t.Fatalf("fetchGiteaToken: %v", err)
}
if got := rec.path(); got != "gitea/creds/unkin-agent" {
t.Errorf("vault read path = %q, want gitea/creds/unkin-agent", got)
}
}
+8 -7
View File
@@ -49,9 +49,10 @@ func approleLogin(vaultAddr, roleID string) (string, error) {
return out.Auth.ClientToken, nil
}
// readGiteaCreds reads the Gitea creds secret and returns the token field.
func readGiteaCreds(vaultAddr, clientToken string) (string, error) {
url := strings.TrimRight(vaultAddr, "/") + "/v1/" + GiteaCredsPath
// readGiteaCreds reads the Gitea creds secret at credsPath and returns the
// token field.
func readGiteaCreds(vaultAddr, clientToken, credsPath string) (string, error) {
url := strings.TrimRight(vaultAddr, "/") + "/v1/" + credsPath
req, err := http.NewRequest(http.MethodGet, url, nil)
if err != nil {
return "", err
@@ -60,12 +61,12 @@ func readGiteaCreds(vaultAddr, clientToken string) (string, error) {
resp, err := httpClient.Do(req)
if err != nil {
return "", fmt.Errorf("vault read %s: %w", GiteaCredsPath, err)
return "", fmt.Errorf("vault read %s: %w", credsPath, err)
}
defer func() { _ = resp.Body.Close() }()
data, _ := io.ReadAll(resp.Body)
if resp.StatusCode != http.StatusOK {
return "", fmt.Errorf("vault read %s: HTTP %d: %s", GiteaCredsPath, resp.StatusCode, strings.TrimSpace(string(data)))
return "", fmt.Errorf("vault read %s: HTTP %d: %s", credsPath, resp.StatusCode, strings.TrimSpace(string(data)))
}
var out struct {
@@ -74,10 +75,10 @@ func readGiteaCreds(vaultAddr, clientToken string) (string, error) {
} `json:"data"`
}
if err := json.Unmarshal(data, &out); err != nil {
return "", fmt.Errorf("vault read %s: decoding response: %w", GiteaCredsPath, err)
return "", fmt.Errorf("vault read %s: decoding response: %w", credsPath, err)
}
if out.Data.Token == "" {
return "", fmt.Errorf("vault read %s: no token field in secret", GiteaCredsPath)
return "", fmt.Errorf("vault read %s: no token field in secret", credsPath)
}
return out.Data.Token, nil
}
+32
View File
@@ -106,8 +106,40 @@ func (c *VaultClient) ReadKV(mount, path string) (map[string]any, error) {
return out.Data.Data, nil
}
// ReadKVOptional is ReadKV but treats a missing secret as empty: a 404 or a
// deleted version (data: null) yields an empty map, not an error. Callers that
// read-modify-write a path that may not exist yet use this.
func (c *VaultClient) ReadKVOptional(mount, path string) (map[string]any, error) {
var out struct {
Data struct {
Data map[string]any `json:"data"`
} `json:"data"`
}
if err := c.do(http.MethodGet, kvDataPath(mount, path), nil, &out); err != nil {
if errors.Is(err, ErrVaultNotFound) {
return map[string]any{}, nil
}
return nil, err
}
if out.Data.Data == nil {
return map[string]any{}, nil
}
return out.Data.Data, nil
}
// WriteKV writes a KV-v2 secret and returns the version it created.
func (c *VaultClient) WriteKV(mount, path string, data map[string]string) (int, error) {
fields := make(map[string]any, len(data))
for k, v := range data {
fields[k] = v
}
return c.WriteKVAny(mount, path, fields)
}
// WriteKVAny writes a KV-v2 secret whose fields are not all strings (so a
// read-modify-write can put back values it did not author) and returns the
// version it created.
func (c *VaultClient) WriteKVAny(mount, path string, data map[string]any) (int, error) {
var out struct {
Data struct {
Version int `json:"version"`
+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)
}
}