19 Commits

Author SHA1 Message Date
benvin cdced6536e Merge pull request 'Find every stale worktree, not just the managed ones' (#14) from benvin/worktree-gc into main
ci/woodpecker/tag/release Pipeline was successful
Reviewed-on: #14
2026-09-12 00:46:39 +10:00
unkin-agent 72a8923c3d Cover the keep gate for unmanaged worktrees
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
--include-unmanaged lifts only the location gate, and nothing proved the keep
gate still held behind it.

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

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

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

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

- retry a rejected request once with a freshly minted token
- abort the watch when the fresh token is rejected too
- poll anonymously when no token can be minted, mint only on a real 401/403
2026-09-09 21:15:46 +10:00
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 4632 additions and 99 deletions
+67 -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,61 @@ 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.
- `agentws prune` discovers worktrees from the worktree root *and* from
`git worktree list` on each source checkout, merging the two so git's own
`locked`/`prunable` flags reach entries the directory scan already found.
Removing a worktree is only safe because the local branch keeps its commits, so
the cases with no branch to fall back on are kept: a detached HEAD carrying
commits on no remote, a locked checkout, or one with a sequencer operation
half-finished (`rebase-merge`, `MERGE_HEAD`, `CHERRY_PICK_HEAD`, …). A directory
whose backing repo is gone is deleted outright, but only ever inside the
worktree root.
- CI "combined status" comes from `/commits/{sha}/status`; an empty head SHA
yields an empty state without an API call.
+135 -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,96 @@ agentws list
agentws rm benvin/my-change
agentws rm ~/.cache/agentws/argocd-apps__benvin-my-change --delete-branch
# Classify every worktree found; dry run unless --yes is given
agentws prune
agentws prune --json
agentws prune --no-fetch
agentws prune --yes
agentws prune --yes --keep-branches
# Remove every managed worktree and prune each source repo
agentws clean
# Print a fresh unkin-agent Gitea token
# Print a fresh Gitea token for the agent login
agentws token
```
### prune
`agentws prune` finds worktrees two ways and merges the results: the managed
directories under the worktree root, and `git worktree list` on every source
checkout — so hand-made worktrees, stale registrations whose directory is gone,
and leftover directories whose source repo was deleted all show up too.
It then decides, per worktree, whether its work is safely upstream:
| Signal (first match wins) | Verdict |
|---|---|
| working tree gone (registration only) | prune the registration |
| backing repo gone | delete the leftover directory |
| locked by `git worktree lock` | keep |
| rebase, merge, cherry-pick, revert or bisect in progress | keep |
| uncommitted, staged or untracked changes | keep |
| branch has an open PR | keep |
| tip contained in `origin/<default>` | remove worktree + local branch |
| every commit patch-equivalent to one in `origin/<default>`'s history | remove worktree + local branch |
| PR merged **and** HEAD contained in the PR's head commit (or in a verified `origin/<branch>`) | remove worktree + local branch |
| PR closed **and** HEAD contained in a verified `origin/<branch>` | remove worktree + local branch |
| detached HEAD carrying commits on no remote | keep |
| anything else | remove worktree, keep the branch |
A branch is deleted only where git proves its commits survive elsewhere. PR
state alone never authorises that: a merged or closed PR whose branch picked up
commits since keeps its branch, because those commits exist nowhere but here.
The delete runs `git branch -d` first so git's own unmerged check is a backstop,
falling back to `-D` only for a proven branch — squash merges keep the guard
tripping even once the work has landed.
Patch equivalence comes from `git cherry`, which these squash-merging repos need
because a merged branch's commits carry different SHAs upstream. It proves the
patches reached the default branch's history at some point — a later revert
still counts — not that they stand at its tip.
`origin/<branch>` counts as evidence only when this run's `git fetch --prune`
succeeded. A tracking ref left over from an earlier fetch may name a branch that
is already gone upstream and is itself due for deletion, so a failed fetch
downgrades those verdicts to `remove` and keeps the branch. Proofs that read
only local objects — containment in `origin/<default>`, patch equivalence, and
containment in a merged PR's head SHA — stand on their own.
Gitea PR state only adds to the git answer: when it cannot be reached, prune
says so and never deletes a branch it could not prove, and a PR listing that
hits the pagination cap is reported rather than read as "no PR". Matching a
branch to its PR uses `head.label`, since Gitea rewrites `head.ref` to
`refs/pull/<n>/head` once the branch is deleted on merge.
The last row is safe only because the local branch keeps the commits, so the
reason names the branch it is relying on. A detached HEAD has no such branch, so
unique commits there are kept instead.
Output is a table (`REPO BRANCH PATH VERDICT REASON`) with every verdict's reason
spelled out, or `--json` for scripting. Neither form needs a terminal.
| Flag | Effect |
|---|---|
| `--yes` | apply the plan; without it nothing is touched |
| `--keep-branches` | remove worktrees only; verdicts print as `remove` |
| `--no-fetch` | judge against the refs already on disk, for offline use |
| `--json` | emit the report as JSON on stdout, notes on stderr |
| `--include-unmanaged` | also remove worktrees outside the worktree root |
| `--include-keep` | dangerous: also remove worktrees classified `keep`, destroying uncommitted and in-progress work |
Without `--include-unmanaged` a hand-made worktree is reported and then skipped,
naming the flag that would remove it. `--include-keep` is the only way past a
`keep`. It leaves the branch, so committed work outlives the worktree, but
`git worktree remove --force` discards a dirty working tree and a paused
rebase's sequencer state without a word, and no branch was carrying those.
A directory under the worktree root is deleted outright only when its git dir
and the repo's shared `.git` are both proven absent by `stat`. When git merely
fails to answer for a checkout, the verdict is `keep` with the error as its
reason: an unread state is never a dead one.
### Auth / credential-helper design
Gitea tokens minted from Vault are short-lived (~1h), so `agentws` never
@@ -113,7 +202,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 +248,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},
+305 -15
View File
@@ -13,6 +13,8 @@
// agentws new <repo> [--branch benvin/<name>] [--from <base-branch>]
// agentws list
// agentws rm <path-or-branch> [--delete-branch]
// agentws prune [--yes] [--keep-branches] [--no-fetch] [--json]
// [--include-unmanaged] [--include-keep]
// agentws clean
// agentws token
// agentws credential get # git credential-helper protocol on stdin
@@ -24,6 +26,7 @@ import (
"io"
"os"
"path/filepath"
"sort"
"strings"
"git.unkin.net/unkin/agent-tools/internal/agent"
@@ -54,6 +57,7 @@ func newRootCmd() *cobra.Command {
newNewCmd(),
newListCmd(),
newRmCmd(),
newPruneCmd(),
newCleanCmd(),
newTokenCmd(),
newCredentialCmd(),
@@ -226,7 +230,14 @@ func newListCmd() *cobra.Command {
return nil
}
for _, w := range managed {
_, _ = fmt.Fprintf(out, "%s\t%s\t%s\n", w.repo, w.branch, w.path)
branch := w.branch
switch {
case w.orphan:
branch = "(orphan)"
case w.inspectErr != nil:
branch = "(unreadable)"
}
_, _ = fmt.Fprintf(out, "%s\t%s\t%s\n", w.repo, branch, w.path)
}
return nil
},
@@ -239,10 +250,31 @@ type managedWt struct {
branch string
path string
srcDir string
// managed is false for worktrees found via `git worktree list` that live
// outside the worktree root, i.e. somebody made them by hand.
managed bool
// detached is true when the worktree has no branch to fall back on, so its
// commits die with the checkout.
detached bool
// locked records git's own "do not remove me" marker.
locked bool
// missing is a registration whose working tree is gone: nothing to inspect,
// nothing to lose.
missing bool
// orphan is a directory under the worktree root whose backing git dir is
// proven gone, so no git state can be read from it ever again.
orphan bool
// inspectErr is set when git refused to answer for a checkout and the reason
// was not a proven-absent backing repo. The state is unknown, never removable.
inspectErr error
}
// managedWorktrees scans the worktree root and resolves each entry's repo and
// branch from git so branch names are accurate (not the sanitized dir name).
// Directories whose backing repo is proven gone are returned as orphans rather
// than dropped, so callers can see (and clean up) the leftovers; a directory git
// merely failed to answer for is returned with its error instead, because an
// unread state must never be mistaken for a dead one.
func managedWorktrees() ([]managedWt, error) {
wr, err := worktreeRoot()
if err != nil {
@@ -261,24 +293,238 @@ func managedWorktrees() ([]managedWt, error) {
continue
}
path := filepath.Join(wr, e.Name())
branch, err := agent.GitCurrentBranch(path)
if err != nil {
continue // not a git worktree; skip
if _, err := os.Stat(filepath.Join(path, ".git")); err != nil {
continue // not a worktree checkout at all
}
srcDir, err := agent.SourceRepoDir(path)
if err != nil {
branch, branchErr := agent.GitCurrentBranch(path)
srcDir, srcErr := agent.SourceRepoDir(path)
if branchErr != nil || srcErr != nil {
entry := managedWt{repo: repoFromDirName(e.Name()), path: path, managed: true}
if gone, err := backingRepoGone(path); err == nil && gone {
entry.orphan = true
} else if branchErr != nil {
entry.inspectErr = branchErr
} else {
entry.inspectErr = srcErr
}
out = append(out, entry)
continue
}
out = append(out, managedWt{
repo: filepath.Base(srcDir),
branch: branch,
path: path,
srcDir: srcDir,
repo: filepath.Base(srcDir),
branch: branch,
path: path,
srcDir: srcDir,
managed: true,
detached: branch == "HEAD",
})
}
return out, nil
}
// backingRepoGone proves, by stat alone, that a worktree directory's backing
// repo no longer exists: its .git file names a git dir that is absent, and the
// repo's shared .git the git dir lived in is absent too. Only that pair licenses
// deleting the directory. Every other outcome — an unreadable .git file, a git
// dir still on disk, a stat that failed for any reason other than "not there",
// or a mere lost registration in a repo that is still present — reports false,
// so a transient or unexplained failure can never be read as "safe to delete".
func backingRepoGone(path string) (bool, error) {
dot := filepath.Join(path, ".git")
info, err := os.Lstat(dot)
if err != nil {
return false, err
}
if info.IsDir() {
return false, nil // a standalone checkout, not a linked worktree
}
data, err := os.ReadFile(dot)
if err != nil {
return false, err
}
rest, ok := strings.CutPrefix(strings.TrimSpace(string(data)), "gitdir:")
if !ok {
return false, fmt.Errorf("%s: not a worktree gitdir pointer", dot)
}
gitDir := strings.TrimSpace(rest)
if gitDir == "" {
return false, fmt.Errorf("%s: empty gitdir", dot)
}
if !filepath.IsAbs(gitDir) {
gitDir = filepath.Join(path, gitDir)
}
// The git dir is "<repo>/.git/worktrees/<name>"; both it and the shared .git
// it sits in must be absent before the repo counts as gone.
for _, dir := range []string{gitDir, filepath.Dir(filepath.Dir(gitDir))} {
if _, err := os.Stat(dir); err == nil {
return false, nil
} else if !os.IsNotExist(err) {
return false, err
}
}
return true, nil
}
// repoFromDirName recovers the repo name from the "<repo>__<branch>" layout used
// under the worktree root, for entries git can no longer answer for.
func repoFromDirName(name string) string {
if repo, _, ok := strings.Cut(name, "__"); ok {
return repo
}
return name
}
// allWorktrees is every worktree prune should consider: the managed ones under
// the worktree root, plus whatever `git worktree list` reports for the repos
// they belong to and for every checkout in the source root. The second source
// finds hand-made worktrees and stale registrations whose directory is gone, and
// carries git's own locked/prunable flags onto the entries the first source
// already found.
func allWorktrees() ([]managedWt, error) {
managed, err := managedWorktrees()
if err != nil {
return nil, err
}
wr, err := worktreeRoot()
if err != nil {
return nil, err
}
out := make([]managedWt, 0, len(managed))
index := map[string]int{}
for _, w := range managed {
index[resolvePath(w.path)] = len(out)
out = append(out, w)
}
for _, srcDir := range sourceRepos(managed) {
wts, err := agent.GitWorktreeList(srcDir)
if err != nil {
continue // not a repo any more, or unreadable; managed entries still stand
}
for _, wt := range wts {
if wt.Bare || sameDir(wt.Path, srcDir) {
continue
}
_, statErr := os.Stat(wt.Path)
entry := managedWt{
repo: filepath.Base(srcDir),
branch: worktreeBranch(wt),
path: wt.Path,
srcDir: srcDir,
managed: underRoot(wt.Path, wr),
detached: wt.Detached,
locked: wt.Locked,
missing: wt.Prunable != "" || os.IsNotExist(statErr),
}
key := resolvePath(wt.Path)
if i, ok := index[key]; ok {
// Keep the managed scan's own view, but adopt the flags only git knows.
out[i].locked = entry.locked
out[i].missing = out[i].missing || entry.missing
out[i].detached = out[i].detached || entry.detached
continue
}
index[key] = len(out)
out = append(out, entry)
}
}
sort.Slice(out, func(i, j int) bool {
if out[i].repo != out[j].repo {
return out[i].repo < out[j].repo
}
return out[i].path < out[j].path
})
return out, nil
}
// resolvePath is a path key that matches however git spells the same directory.
func resolvePath(path string) string {
if p, err := filepath.EvalSymlinks(path); err == nil {
return p
}
return filepath.Clean(path)
}
// worktreeBranch names a worktree's branch, reporting a detached checkout as
// "HEAD" so it reads the same as GitCurrentBranch does.
func worktreeBranch(wt agent.Worktree) string {
if wt.Branch != "" {
return wt.Branch
}
return "HEAD"
}
// sourceRepos is every repo to enumerate worktrees from: the ones the managed
// worktrees point back at, plus every git checkout directly under the source
// root (so a repo with only hand-made worktrees is still covered). Each is
// normalised to its main checkout, because a directory in the source root may
// itself be a linked worktree — enumerating from there would report the repo's
// real checkout as a removable worktree of itself.
func sourceRepos(managed []managedWt) []string {
seen := map[string]bool{}
var dirs []string
add := func(dir string) {
if dir == "" {
return
}
if main, err := agent.SourceRepoDir(dir); err == nil {
dir = main
}
key := resolvePath(dir)
if seen[key] {
return
}
seen[key] = true
dirs = append(dirs, dir)
}
for _, w := range managed {
add(w.srcDir)
}
if sr, err := srcRoot(); err == nil {
if entries, err := os.ReadDir(sr); err == nil {
for _, e := range entries {
if !e.IsDir() {
continue
}
dir := filepath.Join(sr, e.Name())
if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil {
add(dir)
}
}
}
}
sort.Strings(dirs)
return dirs
}
// sameDir compares two paths after resolving symlinks, because git reports
// worktree paths fully resolved while our own paths may not be.
func sameDir(a, b string) bool {
if a == b {
return true
}
ra, errA := filepath.EvalSymlinks(a)
rb, errB := filepath.EvalSymlinks(b)
return errA == nil && errB == nil && ra == rb
}
// underRoot reports whether path sits inside root, comparing resolved paths
// because git hands back worktree paths with symlinks already resolved.
func underRoot(path, root string) bool {
if r, err := filepath.EvalSymlinks(root); err == nil {
root = r
}
if p, err := filepath.EvalSymlinks(path); err == nil {
path = p
}
rel, err := filepath.Rel(root, path)
if err != nil {
return false
}
return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
}
// --- rm -------------------------------------------------------------------
func newRmCmd() *cobra.Command {
@@ -294,7 +540,8 @@ func newRmCmd() *cobra.Command {
if err != nil {
return err
}
return removeWorktree(cmd.OutOrStdout(), wt, deleteBranch)
// Naming one worktree to delete is explicit, so rm keeps the force fallback.
return removeWorktree(cmd.OutOrStdout(), wt, deleteBranch, true)
},
}
cmd.Flags().BoolVar(&deleteBranch, "delete-branch", false, "Also delete the local branch after removing the worktree")
@@ -309,20 +556,35 @@ func resolveWorktree(target string) (managedWt, error) {
}
abs, _ := filepath.Abs(target)
for _, w := range managed {
if w.path == target || w.path == abs || w.branch == target {
if w.path == target || w.path == abs || (w.branch != "" && w.branch == target) {
return w, nil
}
}
return managedWt{}, fmt.Errorf("no managed worktree matching %q (try `agentws list`)", target)
}
func removeWorktree(out io.Writer, wt managedWt, deleteBranch bool) error {
// removeWorktree removes a managed worktree and, when asked, its local branch.
// forceBranch overrides git's unmerged-branch guard, so only a caller that
// proved the commits survive elsewhere may set it.
func removeWorktree(out io.Writer, wt managedWt, deleteBranch, forceBranch bool) error {
switch {
case wt.inspectErr != nil:
// No srcDir to act through and no idea what is in there; --include-keep
// must not turn that into a delete.
return fmt.Errorf("refusing to remove %s: git state unreadable: %w", wt.path, wt.inspectErr)
case wt.orphan:
return removeOrphanDir(out, wt)
case wt.missing:
// The working tree is already gone; only the registration is left.
_, _ = fmt.Fprintf(out, "pruned stale registration %s\n", wt.path)
return agent.GitWorktreePrune(wt.srcDir)
}
if err := agent.GitWorktreeRemove(wt.srcDir, wt.path, true); err != nil {
return err
}
_, _ = fmt.Fprintf(out, "removed worktree %s\n", wt.path)
if deleteBranch {
if err := agent.GitDeleteBranch(wt.srcDir, wt.branch, true); err != nil {
if err := deleteLocalBranch(wt, forceBranch); err != nil {
return err
}
_, _ = fmt.Fprintf(out, "deleted branch %s\n", wt.branch)
@@ -334,6 +596,34 @@ func removeWorktree(out io.Writer, wt managedWt, deleteBranch bool) error {
return agent.GitWorktreePrune(wt.srcDir)
}
// removeOrphanDir deletes a worktree directory whose backing repo is gone. git
// cannot act on it, so this is a plain delete — confined to the worktree root so
// a bad path can never reach a real checkout.
func removeOrphanDir(out io.Writer, wt managedWt) error {
wr, err := worktreeRoot()
if err != nil {
return err
}
if !underRoot(wt.path, wr) || sameDir(wt.path, wr) {
return fmt.Errorf("refusing to delete %s: not inside the worktree root %s", wt.path, wr)
}
if err := os.RemoveAll(wt.path); err != nil {
return err
}
_, _ = fmt.Fprintf(out, "deleted orphaned worktree directory %s\n", wt.path)
return nil
}
// deleteLocalBranch tries the guarded delete first so git refuses to drop
// unmerged commits on its own; force is a fallback, never the first attempt.
func deleteLocalBranch(wt managedWt, force bool) error {
err := agent.GitDeleteBranch(wt.srcDir, wt.branch, false)
if err == nil || !force {
return err
}
return agent.GitDeleteBranch(wt.srcDir, wt.branch, true)
}
// --- clean ----------------------------------------------------------------
func newCleanCmd() *cobra.Command {
@@ -352,7 +642,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
}
}
+480
View File
@@ -0,0 +1,480 @@
package main
import (
"encoding/json"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"sort"
"strings"
"text/tabwriter"
"git.unkin.net/unkin/agent-tools/internal/agent"
"github.com/spf13/cobra"
)
// Verdicts a worktree can be classified into.
const (
verdictKeep = "keep"
verdictRemove = "remove"
verdictRemoveBranch = "remove+branch"
)
// prLister is the slice of the Gitea client prune needs, so tests can drive
// classification without a live server.
type prLister interface {
ListPRs(repoPath, state string) ([]agent.PullRequest, error)
}
type pruneResult struct {
wt managedWt
verdict string
reason string
// proven records that git itself confirmed the branch's commits survive
// elsewhere; only then may a branch delete override git's own guard.
proven bool
}
// repoCtx is the per-repo state classification is decided against.
type repoCtx struct {
srcDir string
defBranch string
prs map[string]agent.PullRequest
prsKnown bool
// fetched records that this run's pruning fetch succeeded; without it an
// origin/<branch> ref may be stale and due for deletion, so it proves nothing.
fetched bool
// unfetched explains why, so a verdict can say which it was.
unfetched string
}
// pruneOpts is the knob set runPrune is driven by.
type pruneOpts struct {
apply bool
keepBranches bool
noFetch bool
jsonOut bool
includeKeep bool
includeUnmanaged bool
}
// reportEntry is the --json shape: one object per worktree, mirroring the table.
type reportEntry struct {
Repo string `json:"repo"`
Branch string `json:"branch"`
Path string `json:"path"`
Verdict string `json:"verdict"`
Reason string `json:"reason"`
Managed bool `json:"managed"`
Applied bool `json:"applied"`
}
func newPruneCmd() *cobra.Command {
var opts pruneOpts
cmd := &cobra.Command{
Use: "prune",
Short: "Classify worktrees and remove the ones whose work is safely upstream",
Long: "prune inspects every worktree it can find — the managed ones under the worktree\nroot plus whatever `git worktree list` reports for the source checkouts — and\nclassifies each against git and its Gitea pull request. It reports and changes\nnothing unless --yes is given.",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
return runPrune(cmd.OutOrStdout(), cmd.ErrOrStderr(), pruneClient(), opts)
},
}
f := cmd.Flags()
f.BoolVar(&opts.apply, "yes", false, "Actually remove worktrees (default is a dry run)")
f.BoolVar(&opts.keepBranches, "keep-branches", false, "Never delete a local branch, whatever the classification")
f.BoolVar(&opts.noFetch, "no-fetch", false, "Do not fetch; judge against the refs already on disk")
f.BoolVar(&opts.jsonOut, "json", false, "Emit JSON instead of a table")
f.BoolVar(&opts.includeKeep, "include-keep", false, "Dangerous: also remove worktrees classified keep (needs --yes). Destroys uncommitted changes and paused rebase/merge state, which no branch is carrying; only the branch itself survives")
f.BoolVar(&opts.includeUnmanaged, "include-unmanaged", false, "Also remove worktrees that live outside the worktree root")
return cmd
}
// pruneClient builds a Gitea client, falling back to anonymous access when no
// token can be minted; prune degrades to git-only signals if that fails too.
func pruneClient() prLister {
tok, err := agent.GiteaToken()
if err != nil {
tok = ""
}
return agent.NewGiteaClient(tok)
}
func runPrune(out, errOut io.Writer, prs prLister, opts pruneOpts) error {
// In JSON mode stdout carries the document alone, so notes go to stderr.
notes := out
if opts.jsonOut {
notes = errOut
}
worktrees, err := allWorktrees()
if err != nil {
return err
}
if len(worktrees) == 0 {
if opts.jsonOut {
_, _ = fmt.Fprintln(out, "[]")
return nil
}
_, _ = fmt.Fprintln(out, "no managed worktrees")
return nil
}
results := classifyAll(notes, prs, worktrees, opts)
if err := report(out, results, opts); err != nil {
return err
}
if !opts.apply {
if !opts.jsonOut {
_, _ = fmt.Fprintln(out, "dry run: nothing removed (pass --yes to apply)")
}
return nil
}
return applyPrune(notes, results, opts)
}
// classifyAll groups worktrees by source repo so each repo is fetched and its
// PRs listed once, then classifies every worktree against that repo's state.
func classifyAll(notes io.Writer, prs prLister, worktrees []managedWt, opts pruneOpts) []pruneResult {
byRepo := map[string][]managedWt{}
var results []pruneResult
for _, w := range worktrees {
switch {
case w.inspectErr != nil:
// Unknown is not gone: a checkout git refused to answer for keeps.
results = append(results, pruneResult{wt: w, verdict: verdictKeep, reason: "inspection failed: " + oneLine(w.inspectErr.Error())})
case w.orphan:
results = append(results, pruneResult{wt: w, verdict: verdictRemove, reason: "backing repo gone, no git state to read"})
default:
byRepo[w.srcDir] = append(byRepo[w.srcDir], w)
}
}
srcDirs := make([]string, 0, len(byRepo))
for dir := range byRepo {
srcDirs = append(srcDirs, dir)
}
sort.Strings(srcDirs)
for _, srcDir := range srcDirs {
ctx, err := newRepoCtx(notes, prs, srcDir, opts.noFetch)
if err != nil {
for _, w := range byRepo[srcDir] {
results = append(results, pruneResult{wt: w, verdict: verdictKeep, reason: "repo state unknown: " + oneLine(err.Error())})
}
continue
}
for _, w := range byRepo[srcDir] {
res, err := classify(w, ctx)
if err != nil {
res = pruneResult{wt: w, verdict: verdictKeep, reason: "inspection failed: " + oneLine(err.Error())}
}
results = append(results, res)
}
}
sort.SliceStable(results, func(i, j int) bool {
if results[i].wt.repo != results[j].wt.repo {
return results[i].wt.repo < results[j].wt.repo
}
return results[i].wt.path < results[j].wt.path
})
return results
}
// report writes the classification as a table or as JSON.
func report(out io.Writer, results []pruneResult, opts pruneOpts) error {
if opts.jsonOut {
entries := make([]reportEntry, 0, len(results))
for _, r := range results {
entries = append(entries, reportEntry{
Repo: r.wt.repo,
Branch: r.wt.branch,
Path: r.wt.path,
Verdict: plannedVerdict(r, opts),
Reason: r.reason,
Managed: r.wt.managed,
Applied: opts.apply && willRemove(r, opts),
})
}
enc := json.NewEncoder(out)
enc.SetIndent("", " ")
return enc.Encode(entries)
}
// Padded with spaces only, so the table reads the same with or without a TTY.
tw := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0)
_, _ = fmt.Fprintln(tw, "REPO\tBRANCH\tPATH\tVERDICT\tREASON")
for _, r := range results {
_, _ = fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\n",
dash(r.wt.repo), dash(r.wt.branch), abbrevHome(r.wt.path), plannedVerdict(r, opts), r.reason)
}
return tw.Flush()
}
// oneLine flattens a git error onto a single line so one row stays one row.
func oneLine(s string) string {
return strings.Join(strings.Fields(s), " ")
}
func dash(s string) string {
if s == "" {
return "-"
}
return s
}
// abbrevHome shortens $HOME to ~ so paths do not dominate the table.
func abbrevHome(path string) string {
home, err := os.UserHomeDir()
if err != nil || home == "" || !strings.HasPrefix(path, home+string(filepath.Separator)) {
return path
}
return "~" + path[len(home):]
}
// applyPrune performs the removals the classification authorised, reporting each
// one and collecting failures so one bad worktree does not stop the rest.
func applyPrune(out io.Writer, results []pruneResult, opts pruneOpts) error {
var errs []error
for _, r := range results {
if !willRemove(r, opts) {
if !r.wt.managed && r.verdict != verdictKeep {
_, _ = fmt.Fprintf(out, "skipped %s: outside the worktree root (pass --include-unmanaged)\n", r.wt.path)
}
continue
}
if r.verdict == verdictKeep {
_, _ = fmt.Fprintf(out, "warn: removing %s despite %q (--include-keep)\n", r.wt.path, r.reason)
}
deleteBranch := plannedVerdict(r, opts) == verdictRemoveBranch
if err := removeWorktree(out, r.wt, deleteBranch, r.proven); err != nil {
errs = append(errs, fmt.Errorf("%s: %w", r.wt.path, err))
}
}
return errors.Join(errs...)
}
// willRemove is the single gate on destruction: a keep verdict needs
// --include-keep, and a worktree outside the worktree root needs
// --include-unmanaged.
func willRemove(r pruneResult, opts pruneOpts) bool {
if !r.wt.managed && !opts.includeUnmanaged {
return false
}
if r.verdict == verdictKeep {
return opts.includeKeep
}
return true
}
// plannedVerdict is what will actually happen, so no flag prints an action it
// will not perform. A detached HEAD has no branch to delete, and a keep forced
// through with --include-keep never takes its branch with it.
func plannedVerdict(r pruneResult, opts pruneOpts) string {
if r.verdict != verdictRemoveBranch {
return r.verdict
}
if opts.keepBranches || r.wt.detached {
return verdictRemove
}
return verdictRemoveBranch
}
// newRepoCtx refreshes a source repo and collects the signals prune classifies
// against. A failed or skipped fetch and an unreachable Gitea are reported and
// tolerated: the signals that hold offline still work, and the rest are recorded
// as unverified.
func newRepoCtx(out io.Writer, prs prLister, srcDir string, noFetch bool) (repoCtx, error) {
ctx := repoCtx{srcDir: srcDir, prs: map[string]agent.PullRequest{}}
repo := filepath.Base(srcDir)
if noFetch {
ctx.unfetched = "fetch skipped"
_, _ = fmt.Fprintf(out, "warn: fetch %s skipped (--no-fetch, remote state unverified)\n", repo)
} else if err := agent.GitFetchPrune(srcDir, "origin", credentialHelperArgs()...); err != nil {
ctx.unfetched = "fetch failed"
_, _ = fmt.Fprintf(out, "warn: fetch %s: %v (remote state unverified)\n", repo, err)
} else {
ctx.fetched = true
}
def, err := agent.GitRemoteDefaultBranch(srcDir, "origin")
if err != nil {
return repoCtx{}, err
}
ctx.defBranch = def
if prs == nil {
return ctx, nil
}
list, err := prs.ListPRs(repoPath(srcDir, repo), "all")
switch {
case errors.Is(err, agent.ErrPRListTruncated):
// A branch missing from a partial listing must not read as "no PR".
_, _ = fmt.Fprintf(out, "warn: list PRs for %s: %v (older PRs unseen)\n", repo, err)
ctx.prs = prsByBranch(list)
case err != nil:
_, _ = fmt.Fprintf(out, "warn: list PRs for %s: %v (git signals only)\n", repo, err)
default:
ctx.prs = prsByBranch(list)
ctx.prsKnown = true
}
return ctx, nil
}
// repoPath is the Gitea "owner/repo" for a checkout, read from origin's URL
// because not every managed repo lives under AGENTWS_OWNER.
func repoPath(srcDir, repo string) string {
url, err := agent.GitRemoteURL(srcDir, "origin")
if err == nil && agent.RemoteHost(url) == giteaHost() {
if path, err := agent.RepoPathFromRemoteURL(url); err == nil {
return path
}
}
return owner() + "/" + repo
}
// prsByBranch indexes PRs by head branch, preferring an open PR and otherwise
// the most recent one when a branch has been used more than once.
func prsByBranch(list []agent.PullRequest) map[string]agent.PullRequest {
out := map[string]agent.PullRequest{}
for _, pr := range list {
branch := agent.PRHeadBranch(pr)
if branch == "" {
continue
}
if cur, ok := out[branch]; ok && !supersedes(pr, cur) {
continue
}
out[branch] = pr
}
return out
}
func supersedes(a, b agent.PullRequest) bool {
if a.IsOpen() != b.IsOpen() {
return a.IsOpen()
}
if a.Merged != b.Merged {
return a.Merged
}
return a.Number > b.Number
}
// headContainedIn reports whether the worktree's HEAD is reachable from ref. A
// ref that cannot be resolved proves nothing, so it reads as not contained.
func headContainedIn(dir, ref string) bool {
if ref == "" {
return false
}
ok, err := agent.GitIsAncestor(dir, "HEAD", ref)
return err == nil && ok
}
// classify applies the prune precedence. Removal must never destroy state that
// exists nowhere else: a vanished working tree is the one case with nothing to
// lose, a locked or mid-rebase checkout holds sequencer state git itself refuses
// to discard, and a dirty checkout or a detached HEAD with unique commits holds
// work no branch is carrying. Past those guards, provably-upstream work loses its
// branch too, and anything unproven keeps its branch so no commit becomes
// unreachable. A PR's state alone never authorises deleting a branch — git must
// confirm HEAD is contained in what merged or in what origin still holds, and
// origin's refs only count when this run's pruning fetch refreshed them.
func classify(wt managedWt, ctx repoCtx) (pruneResult, error) {
res := pruneResult{wt: wt}
if wt.missing {
res.verdict, res.reason = verdictRemove, "working tree gone, stale registration only"
return res, nil
}
if wt.locked {
res.verdict, res.reason = verdictKeep, "locked"
return res, nil
}
op, err := agent.GitInProgressOp(wt.path)
if err != nil {
return res, err
}
if op != "" {
res.verdict, res.reason = verdictKeep, op+" in progress"
return res, nil
}
dirty, err := agent.GitIsDirty(wt.path)
if err != nil {
return res, err
}
if dirty {
res.verdict, res.reason = verdictKeep, "dirty"
return res, nil
}
pr, hasPR := ctx.prs[wt.branch]
if hasPR && pr.IsOpen() {
res.verdict, res.reason = verdictKeep, fmt.Sprintf("PR open #%d", pr.Number)
return res, nil
}
upstream := "origin/" + ctx.defBranch
contained, err := agent.GitIsAncestor(wt.path, "HEAD", upstream)
if err != nil {
return res, err
}
if contained {
res.verdict, res.reason, res.proven = verdictRemoveBranch, "contained in "+upstream, true
return res, nil
}
unmerged, err := agent.GitUnmergedCommits(wt.path, upstream, "HEAD")
if err != nil {
return res, err
}
if unmerged == 0 {
// git cherry proves the patches reached that history, not that they stand at its tip.
res.verdict, res.reason, res.proven = verdictRemoveBranch, "patch-equivalent commits in "+upstream+" history", true
return res, nil
}
local, err := agent.GitCommitsNotOnRemotes(wt.path)
if err != nil {
return res, err
}
if wt.detached && local > 0 {
res.verdict, res.reason = verdictKeep, fmt.Sprintf("detached HEAD carrying %s on no remote", commitCount(local))
return res, nil
}
remote := "origin/" + wt.branch
onOrigin := hasPR && ctx.fetched && agent.GitRemoteBranchExists(ctx.srcDir, "origin", wt.branch)
switch {
case hasPR && pr.Merged && headContainedIn(wt.path, pr.Head.Sha):
res.verdict, res.reason, res.proven = verdictRemoveBranch, fmt.Sprintf("PR merged #%d, HEAD contained in the merged head", pr.Number), true
case hasPR && pr.Merged && onOrigin && headContainedIn(wt.path, remote):
res.verdict, res.reason, res.proven = verdictRemoveBranch, fmt.Sprintf("PR merged #%d, HEAD contained in %s", pr.Number, remote), true
case hasPR && pr.Merged && !ctx.fetched:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR merged #%d, %s so %s is unverified", pr.Number, ctx.unfetched, remote)
case hasPR && pr.Merged:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR merged #%d, local commits not in the merged head", pr.Number)
case hasPR && onOrigin && headContainedIn(wt.path, remote):
res.verdict, res.reason, res.proven = verdictRemoveBranch, fmt.Sprintf("PR closed #%d, HEAD contained in %s", pr.Number, remote), true
case hasPR && onOrigin:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR closed #%d, local commits not on %s", pr.Number, remote)
case hasPR && !ctx.fetched:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR closed #%d, %s so %s is unverified", pr.Number, ctx.unfetched, remote)
case hasPR:
res.verdict, res.reason = verdictRemove, fmt.Sprintf("PR closed #%d, branch gone", pr.Number)
case ctx.prsKnown:
res.verdict, res.reason = verdictRemove, "no PR"
default:
res.verdict, res.reason = verdictRemove, "PR state unknown"
}
if res.verdict == verdictRemove && local > 0 {
res.reason += fmt.Sprintf(", %s on no remote so branch %s is kept", commitCount(local), wt.branch)
}
return res, nil
}
func commitCount(n int) string {
if n == 1 {
return "1 commit"
}
return fmt.Sprintf("%d commits", n)
}
File diff suppressed because it is too large Load Diff
+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)
}
}
+132
View File
@@ -2,10 +2,12 @@ package agent
import (
"bytes"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
)
@@ -24,6 +26,10 @@ type Worktree struct {
Branch string // short branch name ("" when detached or bare)
Bare bool
Detached bool
Locked bool
// Prunable is git's own reason a registration is stale (e.g. "gitdir file
// points to non-existent location"); empty when the worktree is intact.
Prunable string
}
// runGit runs git with args, using dir as the working directory (empty = the
@@ -67,6 +73,14 @@ func GitFetch(repoDir, remote string, globalArgs ...string) error {
return err
}
// GitFetchPrune runs `git fetch --prune <remote>` in repoDir so remote-tracking
// refs for branches deleted on the remote (e.g. after a merge) disappear.
func GitFetchPrune(repoDir, remote string, globalArgs ...string) error {
args := append(append([]string{}, globalArgs...), "fetch", "--prune", remote)
_, err := runGit(repoDir, args...)
return err
}
// GitRemoteDefaultBranch returns the short name of remote's default branch
// (e.g. "main") by resolving refs/remotes/<remote>/HEAD.
func GitRemoteDefaultBranch(repoDir, remote string) (string, error) {
@@ -83,6 +97,112 @@ 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
}
// GitDir returns the absolute path to the git directory backing the checkout at
// dir (per-worktree, unlike GitCommonDir).
func GitDir(dir string) (string, error) {
return runGit(dir, "rev-parse", "--path-format=absolute", "--git-dir")
}
// inProgressMarkers maps a sentinel inside the git dir to the operation it means
// is half-finished. Such a checkout holds state that lives nowhere else.
var inProgressMarkers = []struct{ path, op string }{
{"rebase-merge", "rebase"},
{"rebase-apply", "rebase"},
{"MERGE_HEAD", "merge"},
{"CHERRY_PICK_HEAD", "cherry-pick"},
{"REVERT_HEAD", "revert"},
{"BISECT_LOG", "bisect"},
}
// GitInProgressOp names the sequencer operation underway in the checkout at dir,
// or "" when none is.
func GitInProgressOp(dir string) (string, error) {
gitDir, err := GitDir(dir)
if err != nil {
return "", err
}
for _, m := range inProgressMarkers {
if _, err := os.Stat(filepath.Join(gitDir, m.path)); err == nil {
return m.op, nil
} else if !os.IsNotExist(err) {
return "", err
}
}
return "", nil
}
// GitCommitsNotOnRemotes counts commits reachable from HEAD that no
// remote-tracking ref holds, i.e. work that exists only in this checkout.
func GitCommitsNotOnRemotes(dir string) (int, error) {
out, err := runGit(dir, "rev-list", "--count", "HEAD", "--not", "--remotes")
if err != nil {
return 0, err
}
n, err := strconv.Atoi(strings.TrimSpace(out))
if err != nil {
return 0, fmt.Errorf("parse rev-list count %q: %w", out, err)
}
return n, nil
}
// GitIsAncestor reports whether ancestor is reachable from descendant.
func GitIsAncestor(repoDir, ancestor, descendant string) (bool, error) {
cmd := exec.Command("git", "merge-base", "--is-ancestor", ancestor, descendant)
cmd.Dir = repoDir
var stderr bytes.Buffer
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
// Exit 1 is the documented "not an ancestor" answer; anything else is a
// real failure (bad ref, not a repo).
var exitErr *exec.ExitError
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
return false, nil
}
return false, fmt.Errorf("git merge-base --is-ancestor %s %s: %w: %s",
ancestor, descendant, err, strings.TrimSpace(stderr.String()))
}
return true, nil
}
// GitUnmergedCommits counts commits on head whose patch has no equivalent on
// upstream, using `git cherry` so squash- and rebase-merged work is recognised
// despite its rewritten SHAs.
func GitUnmergedCommits(repoDir, upstream, head string) (int, error) {
out, err := runGit(repoDir, "cherry", upstream, head)
if err != nil {
return 0, err
}
n := 0
for _, line := range strings.Split(out, "\n") {
if strings.HasPrefix(strings.TrimSpace(line), "+") {
n++
}
}
return n, nil
}
// GitWorktreeAdd adds a worktree at path checked out to branch. When the branch
// already exists it is reused; otherwise it is created from startPoint.
func GitWorktreeAdd(repoDir, path, branch, startPoint string) error {
@@ -210,6 +330,18 @@ func ParseWorktreeList(out string) []Worktree {
if cur != nil {
cur.Detached = true
}
case "locked":
if cur != nil {
cur.Locked = true
}
case "prunable":
if cur != nil {
// git omits the reason when it has none, so record the flag itself.
cur.Prunable = val
if cur.Prunable == "" {
cur.Prunable = "prunable"
}
}
}
}
flush()
+238 -2
View File
@@ -42,10 +42,20 @@ branch refs/heads/benvin/foo
worktree /home/ben/.cache/agentws/repo__detached
HEAD 3333333333333333333333333333333333333333
detached
worktree /home/ben/.cache/agentws/repo__gone
HEAD 4444444444444444444444444444444444444444
branch refs/heads/benvin/gone
prunable gitdir file points to non-existent location
worktree /home/ben/.cache/agentws/repo__held
HEAD 5555555555555555555555555555555555555555
branch refs/heads/benvin/held
locked
`
wts := ParseWorktreeList(out)
if len(wts) != 3 {
t.Fatalf("got %d worktrees, want 3: %+v", len(wts), wts)
if len(wts) != 5 {
t.Fatalf("got %d worktrees, want 5: %+v", len(wts), wts)
}
if wts[0].Branch != "main" || wts[0].Path != "/home/ben/src/prodenv/repo" {
t.Errorf("wt[0] = %+v", wts[0])
@@ -56,6 +66,104 @@ detached
if !wts[2].Detached || wts[2].Branch != "" {
t.Errorf("wt[2] = %+v, want detached with empty branch", wts[2])
}
if wts[3].Prunable != "gitdir file points to non-existent location" {
t.Errorf("wt[3].Prunable = %q", wts[3].Prunable)
}
if !wts[4].Locked || wts[4].Prunable != "" {
t.Errorf("wt[4] = %+v, want locked and not prunable", wts[4])
}
}
// A bare "prunable" with no reason still has to read as prunable.
func TestParseWorktreeListPrunableWithoutReason(t *testing.T) {
wts := ParseWorktreeList("worktree /tmp/wt\nHEAD 1111111111111111111111111111111111111111\ndetached\nprunable\n")
if len(wts) != 1 || wts[0].Prunable == "" {
t.Errorf("ParseWorktreeList = %+v, want one prunable worktree", wts)
}
}
func TestGitInProgressOp(t *testing.T) {
srcDir := newTempRepos(t)
gitDir, err := GitDir(srcDir)
if err != nil {
t.Fatalf("GitDir: %v", err)
}
if op, err := GitInProgressOp(srcDir); err != nil || op != "" {
t.Fatalf("clean checkout: op = %q, err = %v", op, err)
}
tests := []struct {
marker string
dir bool
want string
}{
{"MERGE_HEAD", false, "merge"},
{"CHERRY_PICK_HEAD", false, "cherry-pick"},
{"REVERT_HEAD", false, "revert"},
{"BISECT_LOG", false, "bisect"},
{"rebase-merge", true, "rebase"},
{"rebase-apply", true, "rebase"},
}
for _, tt := range tests {
path := filepath.Join(gitDir, tt.marker)
if tt.dir {
if err := os.Mkdir(path, 0o755); err != nil {
t.Fatal(err)
}
} else if err := os.WriteFile(path, []byte("x\n"), 0o644); err != nil {
t.Fatal(err)
}
op, err := GitInProgressOp(srcDir)
if err != nil || op != tt.want {
t.Errorf("%s: op = %q, err = %v; want %q", tt.marker, op, err, tt.want)
}
if err := os.RemoveAll(path); err != nil {
t.Fatal(err)
}
}
}
// Commits that reached a remote-tracking ref are not unique local work; commits
// made after the push are.
func TestGitCommitsNotOnRemotes(t *testing.T) {
srcDir := newTempRepos(t)
if n, err := GitCommitsNotOnRemotes(srcDir); err != nil || n != 0 {
t.Fatalf("freshly cloned main: n = %d, err = %v; want 0", n, err)
}
if _, err := runGit(srcDir, "checkout", "-b", "benvin/x"); err != nil {
t.Fatal(err)
}
commitFile(t, srcDir, "a.txt", "a\n", "local a")
commitFile(t, srcDir, "b.txt", "b\n", "local b")
if n, err := GitCommitsNotOnRemotes(srcDir); err != nil || n != 2 {
t.Fatalf("two unpushed commits: n = %d, err = %v; want 2", n, err)
}
if _, err := runGit(srcDir, "push", "origin", "benvin/x"); err != nil {
t.Fatal(err)
}
if n, err := GitCommitsNotOnRemotes(srcDir); err != nil || n != 0 {
t.Fatalf("after push: n = %d, err = %v; want 0", n, err)
}
commitFile(t, srcDir, "c.txt", "c\n", "local c")
if n, err := GitCommitsNotOnRemotes(srcDir); err != nil || n != 1 {
t.Fatalf("one commit past the push: n = %d, err = %v; want 1", n, err)
}
}
func commitFile(t *testing.T, dir, name, content, msg string) {
t.Helper()
if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
t.Fatal(err)
}
if _, err := runGit(dir, "add", "."); err != nil {
t.Fatal(err)
}
if _, err := runGit(dir, "commit", "-m", msg); err != nil {
t.Fatal(err)
}
}
// gitSeed sets a repo-local identity so commits work without global config.
@@ -185,6 +293,134 @@ func TestGitWorktreeLifecycle(t *testing.T) {
}
}
// commit writes a file and commits it, returning the new HEAD sha.
func commit(t *testing.T, dir, name, content, msg string) string {
t.Helper()
if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
t.Fatal(err)
}
if _, err := runGit(dir, "add", "."); err != nil {
t.Fatalf("add: %v", err)
}
if _, err := runGit(dir, "commit", "-m", msg); err != nil {
t.Fatalf("commit: %v", err)
}
sha, err := runGit(dir, "rev-parse", "HEAD")
if err != nil {
t.Fatalf("rev-parse: %v", err)
}
return sha
}
func TestGitIsAncestor(t *testing.T) {
srcDir := newTempRepos(t)
base, err := runGit(srcDir, "rev-parse", "HEAD")
if err != nil {
t.Fatal(err)
}
tip := commit(t, srcDir, "a.txt", "a\n", "add a")
if ok, err := GitIsAncestor(srcDir, base, tip); err != nil || !ok {
t.Errorf("GitIsAncestor(base, tip) = %v, %v; want true", ok, err)
}
if ok, err := GitIsAncestor(srcDir, tip, base); err != nil || ok {
t.Errorf("GitIsAncestor(tip, base) = %v, %v; want false with no error", ok, err)
}
if _, err := GitIsAncestor(srcDir, "no-such-ref", tip); err == nil {
t.Error("GitIsAncestor with a bogus ref should error, not report false")
}
}
// These repos squash-merge, so merged work keeps its local SHA while the
// upstream commit is a different one carrying the same patch. `git cherry` must
// see that as merged even though the SHAs differ.
func TestGitUnmergedCommitsIgnoresRewrittenSHAs(t *testing.T) {
srcDir := newTempRepos(t)
if _, err := runGit(srcDir, "checkout", "-b", "feature"); err != nil {
t.Fatalf("checkout: %v", err)
}
commit(t, srcDir, "f.txt", "hello\n", "add f")
n, err := GitUnmergedCommits(srcDir, "origin/main", "HEAD")
if err != nil {
t.Fatalf("GitUnmergedCommits: %v", err)
}
if n != 1 {
t.Fatalf("unmerged before upstream landing = %d, want 1", n)
}
// Land the same patch upstream under a different SHA.
if _, err := runGit(srcDir, "checkout", "main"); err != nil {
t.Fatalf("checkout main: %v", err)
}
commit(t, srcDir, "f.txt", "hello\n", "squashed f")
if _, err := runGit(srcDir, "push", "origin", "main"); err != nil {
t.Fatalf("push: %v", err)
}
if err := GitFetchPrune(srcDir, "origin"); err != nil {
t.Fatalf("GitFetchPrune: %v", err)
}
if ok, err := GitIsAncestor(srcDir, "feature", "origin/main"); err != nil || ok {
t.Fatalf("squash-merged branch must not be an ancestor: %v, %v", ok, err)
}
n, err = GitUnmergedCommits(srcDir, "origin/main", "feature")
if err != nil {
t.Fatalf("GitUnmergedCommits: %v", err)
}
if n != 0 {
t.Errorf("unmerged after upstream landing = %d, want 0", n)
}
}
func TestGitIsDirty(t *testing.T) {
srcDir := newTempRepos(t)
if dirty, err := GitIsDirty(srcDir); err != nil || dirty {
t.Fatalf("clean checkout reported dirty=%v, err=%v", dirty, err)
}
if err := os.WriteFile(filepath.Join(srcDir, "scratch.txt"), []byte("wip\n"), 0o644); err != nil {
t.Fatal(err)
}
if dirty, err := GitIsDirty(srcDir); err != nil || !dirty {
t.Errorf("untracked file must count as dirty: dirty=%v, err=%v", dirty, err)
}
}
func TestGitRemoteBranchExists(t *testing.T) {
srcDir := newTempRepos(t)
if !GitRemoteBranchExists(srcDir, "origin", "main") {
t.Error("origin/main should exist")
}
if GitRemoteBranchExists(srcDir, "origin", "benvin/nope") {
t.Error("origin/benvin/nope should not exist")
}
if _, err := runGit(srcDir, "checkout", "-b", "benvin/pushed"); err != nil {
t.Fatalf("checkout: %v", err)
}
commit(t, srcDir, "p.txt", "p\n", "add p")
if _, err := runGit(srcDir, "push", "origin", "benvin/pushed"); err != nil {
t.Fatalf("push: %v", err)
}
if err := GitFetchPrune(srcDir, "origin"); err != nil {
t.Fatalf("GitFetchPrune: %v", err)
}
if !GitRemoteBranchExists(srcDir, "origin", "benvin/pushed") {
t.Error("pushed branch should have a remote-tracking ref")
}
if _, err := runGit(srcDir, "push", "origin", "--delete", "benvin/pushed"); err != nil {
t.Fatalf("delete remote branch: %v", err)
}
if err := GitFetchPrune(srcDir, "origin"); err != nil {
t.Fatalf("GitFetchPrune: %v", err)
}
if GitRemoteBranchExists(srcDir, "origin", "benvin/pushed") {
t.Error("a pruning fetch must drop the tracking ref for a deleted remote branch")
}
}
// resolve canonicalizes a path (temp dirs may live behind symlinks like /var).
func resolve(t *testing.T, p string) string {
t.Helper()
+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)
}
}