Name the cause when a watch stops on an auth failure
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful

A 401/403 was handled as one thing, so watchpr re-minted on every
rejection and reported "token expired" for a permission boundary or an
anonymous run that had no token to expire, sending the reader after the
wrong problem.

Classify a 401/403 as a rejected credential, a permission denial, or a
request that carried no token, and re-mint only the first.
Reject a re-minted empty token instead of replaying anonymously.
Report the classified cause from --once as well as from the watch loop.
Document watchpr's exit behaviour per cause.
This commit is contained in:
2026-09-19 16:13:38 +10:00
parent 72adebbf8b
commit 90ce747a61
6 changed files with 387 additions and 17 deletions
+20 -8
View File
@@ -105,7 +105,7 @@ func runOnce(c *agent.GiteaClient, refs []agent.PRRef, jsonMode bool) error {
for _, ref := range refs {
st, err := agent.FetchState(c, ref, login)
if err != nil {
return err
return describeFailure(err)
}
states = append(states, st)
}
@@ -137,18 +137,30 @@ func runWatch(c *agent.GiteaClient, refs []agent.PRRef, interval time.Duration,
res, err := agent.Watch(c, refs, login, ticker.C, onBaseline, onError)
if err != nil {
if agent.IsAuthError(err) {
return fmt.Errorf("gitea authentication failed after re-minting the token, watch aborted: %w", err)
}
if agent.IsPRGone(err) {
return fmt.Errorf("PR no longer visible (repo deleted, renamed, or made private), watch aborted: %w", err)
}
return err
return describeFailure(err)
}
report(res.Ref.String(), res.Reason, res.State, jsonMode)
return nil
}
// describeFailure names the cause of a terminal failure so a watcher that stops
// says why. An anonymous rejection, a permission boundary and a token that
// outlived its Vault lease are three different problems and only the last is
// fixed by a fresh token.
func describeFailure(err error) error {
switch {
case agent.IsNoCredential(err):
return fmt.Errorf("gitea requires authentication and no token could be minted, aborted: %w", err)
case agent.IsPermissionDenied(err):
return fmt.Errorf("gitea denied access to %s (a fresh token will not help), aborted: %w", agent.AgentLogin(), err)
case agent.IsAuthError(err):
return fmt.Errorf("gitea rejected the token and re-minting did not recover it, aborted: %w", err)
case agent.IsPRGone(err):
return fmt.Errorf("PR no longer visible (repo deleted, renamed, or made private), aborted: %w", err)
}
return err
}
// report emits the change that ended the watch.
func report(key, reason string, st agent.PRState, jsonMode bool) {
if jsonMode {
+77
View File
@@ -1,11 +1,15 @@
package main
import (
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"git.unkin.net/unkin/agent-tools/internal/agent"
)
// A bad PR reference must fail the command (so main exits non-zero) rather than
@@ -105,3 +109,76 @@ func TestExecuteBadIntervalErrors(t *testing.T) {
}
}
}
// failingVault serves an AppRole login that never issues a token, so the
// command falls back to anonymous polling exactly as it does when Vault is
// unreachable.
func failingVault(t *testing.T) *httptest.Server {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
}))
t.Cleanup(srv.Close)
return srv
}
// An anonymous run against a repo that is not public must exit non-zero saying
// no token was available — not claim a token expired, and not keep going.
func TestOnceAnonymousRejectionNamesTheMissingToken(t *testing.T) {
vault := failingVault(t)
requests := 0
gitea := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requests++
w.WriteHeader(http.StatusUnauthorized)
_, _ = io.WriteString(w, `{"message":"invalid username, password or token"}`)
}))
defer gitea.Close()
t.Setenv("VAULT_ADDR", vault.URL)
t.Setenv("GITEA_URL", gitea.URL)
cmd := newRootCmd()
cmd.SetArgs([]string{"--once", "unkin/repo#7"})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if err == nil {
t.Fatal("Execute() = nil, want a non-zero exit when the poll is rejected")
}
if !strings.Contains(err.Error(), "no token could be minted") {
t.Errorf("Execute() error = %q, want it to name the missing token", err)
}
if requests != 1 {
t.Errorf("gitea requests = %d, want 1 (no replay without a credential)", requests)
}
}
// describeFailure must tell the four terminal causes apart: each one sends the
// reader somewhere different, and a watcher that stops without saying why is
// the failure this names.
func TestDescribeFailureNamesTheCause(t *testing.T) {
rejected := &agent.APIError{Method: "GET", Path: "/p", StatusCode: 401, Body: `{"message":"invalid username, password or token"}`}
forbidden := &agent.APIError{Method: "GET", Path: "/p", StatusCode: 403, Body: `{"message":"Forbidden"}`}
tests := []struct {
name string
err error
want string
}{
{"anonymous", fmt.Errorf("%w: %w", agent.ErrNoCredential, rejected), "no token could be minted"},
{"permission boundary", error(forbidden), "denied access"},
{"rejected token", error(rejected), "re-minting did not recover it"},
{"other", errors.New("dial tcp: timeout"), "dial tcp: timeout"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := describeFailure(tt.err)
if got == nil || !strings.Contains(got.Error(), tt.want) {
t.Errorf("describeFailure = %v, want it to mention %q", got, tt.want)
}
if !errors.Is(got, tt.err) {
t.Errorf("describeFailure dropped the underlying error %v", tt.err)
}
})
}
}