Skip to content

Commit e581504

Browse files
feat(status): add pending_as_failure option to report failure instead of pending
When a PR doesn't meet approval policy requirements but hasn't been explicitly disapproved, policy-bot normally reports a "pending" GitHub status. This breaks tools like Kodiak that rely on definitive pass/fail status checks and remain stuck waiting for the status to resolve. This change adds a new `pending_as_failure` option that reports "failure" instead of "pending" for such PRs. Configuration options: 1. Server-level (policy-bot.yml): ```yaml options: pending_as_failure: true ``` 2. Environment variable: ``` POLICYBOT_OPTIONS_PENDING_AS_FAILURE=true ``` 3. Per-repo policy override (.policy.yml): ```yaml pending_as_failure: true # or false to override server setting ``` The policy-level setting takes precedence over the server-level setting, allowing repos to opt-in or opt-out individually. Changes: - server/handler/eval_options.go: Add PendingAsFailure field and env var - policy/policy.go: Add PendingAsFailure to Config struct for per-repo override - server/handler/eval_context.go: Check both settings when mapping StatusPending - config/policy-bot.example.yml: Document the new option - server/handler/eval_options_test.go: Add env var test - server/handler/eval_context_test.go: Add comprehensive tests for all scenarios Co-Authored-By: Claude <noreply@anthropic.com>
1 parent b02d577 commit e581504

6 files changed

Lines changed: 212 additions & 1 deletion

File tree

config/policy-bot.example.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,14 @@ sessions:
148148
# # environment variable.
149149
# force_shared_policy: false
150150
#
151+
# # If true, report "failure" instead of "pending" for PRs that don't
152+
# # meet approval requirements. Useful for tools like Kodiak that require
153+
# # definitive pass/fail status checks. This can be overridden per-repo
154+
# # by setting pending_as_failure in the .policy.yml file.
155+
# # Can also be set by the POLICYBOT_OPTIONS_PENDING_AS_FAILURE
156+
# # environment variable.
157+
# pending_as_failure: false
158+
#
151159
# # Default options to use for all appoval rules processed by the server.
152160
# # Policies can override these values with their own defaults or in individual
153161
# # rules. This is equivalent to the "approval_defaults" option in a policy and

policy/policy.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ type Config struct {
3838
Policy Policy `yaml:"policy,omitempty"`
3939
ApprovalDefaults *approval.Defaults `yaml:"approval_defaults,omitempty"`
4040
ApprovalRules []*approval.Rule `yaml:"approval_rules,omitempty"`
41+
42+
// PendingAsFailure reports "failure" instead of "pending" for PRs that
43+
// don't meet approval requirements. When set, this overrides the
44+
// server-level setting.
45+
PendingAsFailure *bool `yaml:"pending_as_failure,omitempty"`
4146
}
4247

4348
type Policy struct {

server/handler/eval_context.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,16 @@ func (ec *EvalContext) EvaluatePolicy(ctx context.Context, evaluator common.Eval
145145
case common.StatusDisapproved:
146146
statusState = "failure"
147147
case common.StatusPending:
148-
statusState = "pending"
148+
// Policy-level setting takes precedence over server-level
149+
pendingAsFailure := ec.Options.PendingAsFailure
150+
if ec.Config.Config != nil && ec.Config.Config.PendingAsFailure != nil {
151+
pendingAsFailure = *ec.Config.Config.PendingAsFailure
152+
}
153+
if pendingAsFailure {
154+
statusState = "failure"
155+
} else {
156+
statusState = "pending"
157+
}
149158
case common.StatusSkipped:
150159
statusState = "error"
151160
statusDescription = "All rules were skipped. At least one rule must match."
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// Copyright 2025 Palantir Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package handler
16+
17+
import (
18+
"context"
19+
"testing"
20+
21+
"github.com/palantir/policy-bot/policy"
22+
"github.com/palantir/policy-bot/policy/common"
23+
"github.com/palantir/policy-bot/pull"
24+
"github.com/palantir/policy-bot/pull/pulltest"
25+
"github.com/stretchr/testify/assert"
26+
"github.com/stretchr/testify/require"
27+
)
28+
29+
type staticEvaluator common.Result
30+
31+
func (eval *staticEvaluator) Trigger() common.Trigger {
32+
return common.TriggerStatic
33+
}
34+
35+
func (eval *staticEvaluator) Evaluate(ctx context.Context, prctx pull.Context) common.Result {
36+
return common.Result(*eval)
37+
}
38+
39+
func TestEvalContext_EvaluatePolicy_PendingAsFailure(t *testing.T) {
40+
ctx := context.Background()
41+
42+
tests := map[string]struct {
43+
serverPendingAsFailure bool
44+
policyPendingAsFailure *bool
45+
resultStatus common.EvaluationStatus
46+
expectedState string
47+
}{
48+
"default_pending_returns_pending": {
49+
serverPendingAsFailure: false,
50+
policyPendingAsFailure: nil,
51+
resultStatus: common.StatusPending,
52+
expectedState: "pending",
53+
},
54+
"server_option_enabled_returns_failure": {
55+
serverPendingAsFailure: true,
56+
policyPendingAsFailure: nil,
57+
resultStatus: common.StatusPending,
58+
expectedState: "failure",
59+
},
60+
"policy_option_enabled_returns_failure": {
61+
serverPendingAsFailure: false,
62+
policyPendingAsFailure: ptr(true),
63+
resultStatus: common.StatusPending,
64+
expectedState: "failure",
65+
},
66+
"policy_option_overrides_server_to_disable": {
67+
serverPendingAsFailure: true,
68+
policyPendingAsFailure: ptr(false),
69+
resultStatus: common.StatusPending,
70+
expectedState: "pending",
71+
},
72+
"approved_returns_success_regardless": {
73+
serverPendingAsFailure: true,
74+
policyPendingAsFailure: nil,
75+
resultStatus: common.StatusApproved,
76+
expectedState: "success",
77+
},
78+
"disapproved_returns_failure_regardless": {
79+
serverPendingAsFailure: false,
80+
policyPendingAsFailure: nil,
81+
resultStatus: common.StatusDisapproved,
82+
expectedState: "failure",
83+
},
84+
"skipped_returns_error_regardless": {
85+
serverPendingAsFailure: true,
86+
policyPendingAsFailure: nil,
87+
resultStatus: common.StatusSkipped,
88+
expectedState: "error",
89+
},
90+
"both_server_and_policy_enabled": {
91+
serverPendingAsFailure: true,
92+
policyPendingAsFailure: ptr(true),
93+
resultStatus: common.StatusPending,
94+
expectedState: "failure",
95+
},
96+
}
97+
98+
for name, test := range tests {
99+
t.Run(name, func(t *testing.T) {
100+
prctx := &pulltest.Context{
101+
OwnerValue: "testowner",
102+
RepoValue: "testrepo",
103+
NumberValue: 1,
104+
HeadSHAValue: "abc123",
105+
BranchBaseName: "main",
106+
}
107+
108+
policyConfig := &policy.Config{}
109+
policyConfig.PendingAsFailure = test.policyPendingAsFailure
110+
111+
ec := &EvalContext{
112+
Options: &PullEvaluationOptions{
113+
StatusCheckContext: "policy-bot",
114+
PendingAsFailure: test.serverPendingAsFailure,
115+
},
116+
PublicURL: "http://localhost:8080",
117+
PullContext: prctx,
118+
Config: FetchedConfig{
119+
Config: policyConfig,
120+
Source: "test",
121+
Path: ".policy.yml",
122+
},
123+
SkipPostStatus: true,
124+
}
125+
126+
evaluator := &staticEvaluator{
127+
Status: test.resultStatus,
128+
StatusDescription: "test description",
129+
}
130+
131+
_, err := ec.EvaluatePolicy(ctx, evaluator)
132+
require.NoError(t, err)
133+
134+
require.NotNil(t, ec.Status, "status should be set")
135+
assert.Equal(t, test.expectedState, *ec.Status.State, "expected state %q, got %q", test.expectedState, *ec.Status.State)
136+
})
137+
}
138+
}
139+
140+
func TestEvalContext_EvaluatePolicy_PendingAsFailure_NilConfig(t *testing.T) {
141+
ctx := context.Background()
142+
143+
prctx := &pulltest.Context{
144+
OwnerValue: "testowner",
145+
RepoValue: "testrepo",
146+
NumberValue: 1,
147+
HeadSHAValue: "abc123",
148+
BranchBaseName: "main",
149+
}
150+
151+
// Test with nil Config in FetchedConfig
152+
ec := &EvalContext{
153+
Options: &PullEvaluationOptions{
154+
StatusCheckContext: "policy-bot",
155+
PendingAsFailure: true,
156+
},
157+
PublicURL: "http://localhost:8080",
158+
PullContext: prctx,
159+
Config: FetchedConfig{
160+
Config: nil, // nil config
161+
Source: "test",
162+
Path: ".policy.yml",
163+
},
164+
SkipPostStatus: true,
165+
}
166+
167+
evaluator := &staticEvaluator{
168+
Status: common.StatusPending,
169+
StatusDescription: "test description",
170+
}
171+
172+
_, err := ec.EvaluatePolicy(ctx, evaluator)
173+
require.NoError(t, err)
174+
175+
require.NotNil(t, ec.Status, "status should be set")
176+
assert.Equal(t, "failure", *ec.Status.State, "server option should be used when policy config is nil")
177+
}

server/handler/eval_options.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ type PullEvaluationOptions struct {
6060
// context behaviour, and will be removed in 2.0
6161
PostInsecureStatusChecks bool `yaml:"post_insecure_status_checks"`
6262

63+
// PendingAsFailure reports "failure" instead of "pending" for PRs that
64+
// don't meet approval requirements. This is useful for tools like Kodiak
65+
// that require definitive pass/fail status checks.
66+
PendingAsFailure bool `yaml:"pending_as_failure"`
67+
6368
// IgnoreEditedComments enables ignoring comments that have been edited when evaluating approval rules.
6469
// This provides a server-side option to ignore edited comments across all rules.
6570
IgnoreEditedComments *bool `yaml:"ignore_edited_comments"`
@@ -118,6 +123,7 @@ func (p *PullEvaluationOptions) SetValuesFromEnv(prefix string) {
118123
setBoolFromEnv("EXPAND_REQUIRED_REVIEWERS", prefix, &p.ExpandRequiredReviewers)
119124
setBoolFromEnv("STRICT_REVIEW_DISMISSAL", prefix, &p.StrictReviewDismissal)
120125
setBoolFromEnv("POST_INSECURE_STATUS_CHECKS", prefix, &p.PostInsecureStatusChecks)
126+
setBoolFromEnv("PENDING_AS_FAILURE", prefix, &p.PendingAsFailure)
121127
setBoolPtrFromEnv("IGNORE_EDITED_COMMENTS", prefix, &p.IgnoreEditedComments)
122128

123129
p.setApprovalDefaultsFromEnv(prefix + "APPROVAL_DEFAULTS_")

server/handler/eval_options_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ func TestPullEvaluationOptions_SetValuesFromEnv(t *testing.T) {
7777
opts.PostInsecureStatusChecks = true
7878
},
7979
},
80+
"PendingAsFailure": {
81+
Env: map[string]string{"PEO_PENDING_AS_FAILURE": "true"},
82+
SetExpected: func(opts *PullEvaluationOptions) {
83+
opts.PendingAsFailure = true
84+
},
85+
},
8086
"IgnoreEditedComments": {
8187
Env: map[string]string{"PEO_IGNORE_EDITED_COMMENTS": "true"},
8288
SetExpected: func(opts *PullEvaluationOptions) {

0 commit comments

Comments
 (0)