LoadError posts failing policy-bot status in repos with no policy

[dblinkhorn]

When policy-bot has installation access to a repo, it attempts to discover the repo's policy before evaluating a PR. If no policy exists, the current behavior is to skip posting status. But if policy discovery hits a non-404 GitHub API error, policy-bot posts a failing error status. This causes noisy failures in repos that do not define a policy and do not require policy-bot as a check.

The README states that a missing policy should not post a status:

If a policy does not exist in the repository or in the shared organization
repository, `policy-bot` does not post a status check on the pull request.

The error noise in repos that don't utilize policy-bot is caused by this code in eval_context.go:

case fc.LoadError != nil:
    msg := fmt.Sprintf("Error loading policy from %s", fc.Source)
    logger.Warn().Err(fc.LoadError).Msg(msg)

    ec.PostStatus(ctx, "error", msg)
    return nil, errors.Wrapf(fc.LoadError, "failed to load policy: %s: %s", fc.Source, fc.Path)

Ideally, the fix would distinguish “missing policy” from “unknown due to API error” without reporting failures in repos that don’t use policy-bot. One option is to stop posting a failing status on LoadError entirely, but where policy-bot is required, users will see policy-bot not reporting/completing for that commit, without any error details provided. Another option is to post LoadError status only for repos/branches where policy-bot is required by branch protection/rulesets.

Hoping to get some advice/discussion here from maintainers. Thanks!

[bluekeyes]

You've identified the core conflict here: if we fail to determine whether the repository has a policy or not, it's hard to know if posting an error status will be helpful or noise. The information we need to make that decision is what's missing because of the failure.

Do you have a specific error that caused this problem? The idea of checking for Policy Bot as a required status is interesting, but requires at least one API call. If the calls to get the policy file contents failed, I'm not sure how likely it is that calls to get branch protection details will succeed. At the same time, the call to post the status worked or this wouldn't be an issue, so there's evidence that the errors can be inconsistent between API calls.

I don't remember if we do any retries in our client currently, but that might be another option. It doesn't solve the problem, but it does make a temporary error while fetching the policy less likely to result in an error status.

I'm also open to making this a sever option. In our internal deployment, the expectation is that every repository has a policy file, so this is a useful error. In deployments where Policy Bot only applies to a few repositories, having an option to suppress this error or assume no policy by default could help.

[dblinkhorn]

Thanks for your reply. There are retries in place for certain errors, but we were seeing 401s which don't get a retry and immediately trigger LoadError. More retries could possibly help for certain errors, but probably would not fully solve the issue.

On our end we have policy-bot installed at the org level with access to all repos by default. This is convenient for when repo owners want to onboard to using policy-bot--no changes need to be made to the GitHub app.

When this issue happens, we see this error in the policy-bot UI:

and this in the GitHub UI:

As far as handling this on the server, what do you think of implementing a small “seen policy” cache with keys like owner/repo/branch? When policy-bot successfully loads a config (fc.Config != nil), insert the new key into the cache. Then when we get one of these LoadError events, we check if the key exists and post the error status if it does, otherwise suppress it.

type SeenPolicyKey struct {
    Owner       string
    Repo        string
    BaseBranch  string
}

type SeenPolicyMetadata struct {
    SeenAt  time.Time
    Source  string    // e.g. "owner/repo@main"
    Path    string
}