From 624f0da4e0888532b8dd3acdba83e4f7baf187ae Mon Sep 17 00:00:00 2001 From: Andrew Pierno Date: Thu, 9 Jul 2026 16:13:13 -0700 Subject: [PATCH 1/3] fix(policy-bot): Retry GitHub 502 responses when loading policy api.github.com intermittently returns 502 on GET /repos/{owner}/{repo}/contents/.policy.yml. isServerError only treated 500, 503 and 504 as retryable, so a single 502 skipped the backoff loop entirely, surfaced as a LoadError, and posted a red "Error loading policy from /@main" status. The underlying httpcache entry is also evicted on any non-200, so the next request could not fall back to the last known good policy. Observed on the wonderlydotcom installation: six load errors in 100 minutes across backend-net and internal-tool-camp, every one a 502, each self-healing on the next webhook. A merge_group receives a single checks_requested delivery, so a 502 there leaves the status red and boots the PR from the queue. 429 is deliberately not added: go-github converts 429 and rate-limited 403 into *RateLimitError / *AbuseRateLimitError, which never satisfy errors.As(err, &ghErr), so such a case would be unreachable. Also corrects a stale expectation in TestParseConfigPostsStatusForSeenPolicy, which still asserted the "policy-bot: main" status context after this branch dropped the branch suffix. It was failing before this change. --- server/handler/eval_context_test.go | 2 +- server/handler/fetcher.go | 2 +- server/handler/fetcher_test.go | 53 +++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/server/handler/eval_context_test.go b/server/handler/eval_context_test.go index 162d1c759..037e74a25 100644 --- a/server/handler/eval_context_test.go +++ b/server/handler/eval_context_test.go @@ -42,7 +42,7 @@ func TestParseConfigPostsStatusForSeenPolicy(t *testing.T) { assert.Nil(t, evaluator) require.NotNil(t, ec.Status) assert.Equal(t, "error", ec.Status.GetState()) - assert.Equal(t, "policy-bot: main", ec.Status.GetContext()) + assert.Equal(t, "policy-bot", ec.Status.GetContext()) assert.Equal(t, "Error loading policy from testorg/testrepo@main", ec.Status.GetDescription()) } diff --git a/server/handler/fetcher.go b/server/handler/fetcher.go index c00aab654..83d4d57c9 100644 --- a/server/handler/fetcher.go +++ b/server/handler/fetcher.go @@ -109,7 +109,7 @@ func isServerError(err error) bool { var ghErr *github.ErrorResponse if errors.As(err, &ghErr) { switch ghErr.Response.StatusCode { - case http.StatusInternalServerError, http.StatusServiceUnavailable, http.StatusGatewayTimeout: + case http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout: return true } } diff --git a/server/handler/fetcher_test.go b/server/handler/fetcher_test.go index 47e548fb3..5d2758075 100644 --- a/server/handler/fetcher_test.go +++ b/server/handler/fetcher_test.go @@ -17,6 +17,7 @@ package handler import ( "context" "errors" + "net/http" "testing" "github.com/google/go-github/v85/github" @@ -112,3 +113,55 @@ func TestConfigFetcherScopesSeenPolicyByBranch(t *testing.T) { require.Error(t, fc.LoadError) assert.False(t, fc.SeenPolicy) } + +func githubStatusError(code int) error { + return &github.ErrorResponse{ + Response: &http.Response{StatusCode: code}, + } +} + +func TestIsServerError(t *testing.T) { + for _, test := range []struct { + code int + retryable bool + }{ + {http.StatusInternalServerError, true}, + {http.StatusBadGateway, true}, + {http.StatusServiceUnavailable, true}, + {http.StatusGatewayTimeout, true}, + {http.StatusForbidden, false}, + {http.StatusNotFound, false}, + {http.StatusUnprocessableEntity, false}, + } { + assert.Equal(t, test.retryable, isServerError(githubStatusError(test.code)), "status %d", test.code) + } + + assert.False(t, isServerError(errors.New("request failed"))) +} + +func TestConfigFetcherRetriesBadGateway(t *testing.T) { + calls := 0 + + fetcher := ConfigFetcher{ + Loader: mockConfigLoader{ + loadConfig: func(ctx context.Context, client *github.Client, owner, repo, ref string) (appconfig.Config, error) { + calls++ + if calls == 1 { + return appconfig.Config{}, githubStatusError(http.StatusBadGateway) + } + return appconfig.Config{ + Content: []byte("policy:\n approval:\n - rule\n"), + Source: "testorg/testrepo@main", + Path: ".policy.yml", + }, nil + }, + }, + SeenPolicyCache: NewSeenPolicyCache(), + } + + fc := fetcher.ConfigForRepositoryBranch(context.Background(), nil, "testorg", "testrepo", "main") + require.NoError(t, fc.LoadError) + require.NoError(t, fc.ParseError) + assert.Equal(t, 2, calls) + assert.True(t, fc.SeenPolicy) +} From abe45cd6e421174d6cbc285f59e1ac637587d54b Mon Sep 17 00:00:00 2001 From: Andrew Pierno Date: Thu, 9 Jul 2026 16:17:37 -0700 Subject: [PATCH 2/3] chore(policy-bot): Remove unused ptr test helper golangci-lint (unused) fails Verify on this branch: policy/approval/approve_test.go:34:6. The helper has zero references repo-wide and was orphaned by 2872525. --- policy/approval/approve_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/policy/approval/approve_test.go b/policy/approval/approve_test.go index 2c9270212..8e1c03dce 100644 --- a/policy/approval/approve_test.go +++ b/policy/approval/approve_test.go @@ -31,10 +31,6 @@ import ( "github.com/stretchr/testify/require" ) -func ptr[T any](v T) *T { - return &v -} - var defaultOptions = Options{ Methods: DefaultMethods(), } From 97515be7b21164dea26a3cfc7f044634ccd17fbd Mon Sep 17 00:00:00 2001 From: Andrew Pierno Date: Thu, 9 Jul 2026 16:22:35 -0700 Subject: [PATCH 3/3] fix(ci): Skip GCR login and image push on pull requests The Dist job's comment says these steps should only run when publishing, but the guard was lost when Docker Hub was swapped for GCR. secrets.GCR_JSON_KEY is not exposed to pull_request events, so docker/login-action fails with 'Password required' on every PR. Dist has failed on all PRs since 2026-04-02, including PRs opened from usemotion-patches itself. Upstream guards the equivalent steps with 'github.event_name == push || github.event_name == release'. Excluding only pull_request preserves the current push and release behaviour. --- .github/workflows/build.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2a795c01f..e84896090 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -114,6 +114,7 @@ jobs: # Include them here to avoid exporting the Docker container as an artifact # - name: Login to GCR + if: ${{ github.event_name != 'pull_request' }} uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4 with: registry: gcr.io @@ -121,6 +122,7 @@ jobs: password: ${{ secrets.GCR_JSON_KEY }} - name: Push release image to gcr + if: ${{ github.event_name != 'pull_request' }} run: ./godelw docker push --tags=latest,version ci-all: