Skip to content

Commit 09033d1

Browse files
wesmcodex
andauthored
Allow selective quiet-hours throttle bypasses (#977)
Quiet-hours throttling currently applies to every PR author, leaving operators with no selective exception short of disabling the stronger interval. This adds a case-insensitive bypass_users list under ci.quiet_hours. The quiet-hours and ordinary throttle exemptions remain independent, so bypassing the overnight layer does not weaken the base per-PR throttle unless both policies exempt the author. The GitHub integration guide documents the distinction, and regression coverage pins both policy combinations. --------- Co-authored-by: Codex <codex@openai.com>
1 parent 3812270 commit 09033d1

5 files changed

Lines changed: 91 additions & 14 deletions

File tree

docs/integrations/github.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,9 +474,10 @@ start = "23:00" # HH:MM, 24-hour clock; both start and end are requir
474474
end = "05:00" # start > end wraps past midnight
475475
timezone = "US/Central" # IANA name; empty means machine local time
476476
throttle_interval = "1h" # per-PR minimum between reviews in the window; default 1h
477+
bypass_users = ["trusted-contributor"] # skips only the quiet-hours throttle
477478
```
478479

479-
While the window is active, the effective throttle for every PR is the larger of `throttle_interval` and `quiet_hours.throttle_interval`, and it applies to **all** users — including `throttle_bypass_users`. A PR's first-ever review is never blocked, and throttled pushes get the usual pending "review deferred" status. When the window ends, the next poll reviews the latest HEAD once, so overnight pushes collapse into a single fresh review.
480+
While the window is active, the effective throttle for every PR is the larger of `throttle_interval` and `quiet_hours.throttle_interval`. The quiet-hours interval applies to every author except those listed in `quiet_hours.bypass_users`; matching is case-insensitive. The ordinary throttle remains independent, so a quiet-hours bypass user must also appear in `[ci].throttle_bypass_users` to bypass both intervals. A PR's first-ever review is never blocked, and throttled pushes get the usual pending "review deferred" status. When the window ends, the next poll reviews the latest HEAD once, so overnight pushes collapse into a single fresh review.
480481

481482
A push deferred only by quiet hours (one the base throttle would have allowed) does not cancel an in-flight review — unlike ordinary throttling, where a new push supersedes the stale run. Frequent overnight pushes would otherwise kill every review before it completes; instead, the running review finishes and posts, so a busy PR gets one snapshot review per interval. Pushes deferred by the base throttle keep their existing supersede behavior, inside or outside the window.
482483

@@ -564,7 +565,8 @@ Set under `[ci.quiet_hours]` (global config only). See [Quiet Hours](#quiet-hour
564565
| `start` | string | | Window start as `"HH:MM"` (24-hour clock). Both `start` and `end` must be set to enable quiet hours. |
565566
| `end` | string | | Window end as `"HH:MM"`. When `start > end` the window wraps past midnight. |
566567
| `timezone` | string | machine local | IANA timezone name (e.g. `"US/Central"`) in which the window is evaluated |
567-
| `throttle_interval` | string | `"1h"` | Per-PR minimum time between reviews while the window is active. Applies to all users, including `throttle_bypass_users`. |
568+
| `throttle_interval` | string | `"1h"` | Per-PR minimum time between reviews while the window is active. |
569+
| `bypass_users` | array | `[]` | GitHub usernames that bypass only the additional quiet-hours throttle (case-insensitive) |
568570

569571
### GitHub App Options
570572

internal/config/ci.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,10 @@ type QuietHoursConfig struct {
325325
// the window is active. Default: "1h". "0" makes quiet hours a
326326
// no-op (a zero interval never exceeds the base throttle).
327327
ThrottleInterval string `toml:"throttle_interval"`
328+
329+
// BypassUsers lists GitHub usernames that bypass the additional
330+
// quiet-hours throttle. Matching is case-insensitive.
331+
BypassUsers []string `toml:"bypass_users"`
328332
}
329333

330334
// QuietHoursWindow is a parsed, validated quiet-hours window.
@@ -410,18 +414,28 @@ func parseClockMinutes(s string) (int, error) {
410414
return t.Hour()*60 + t.Minute(), nil
411415
}
412416

413-
// IsThrottleBypassed reports whether the given GitHub login is in
414-
// the ThrottleBypassUsers list. Comparison is case-insensitive.
415-
func (c *CIConfig) IsThrottleBypassed(login string) bool {
417+
// IsBypassed reports whether the given GitHub login bypasses the additional
418+
// quiet-hours throttle. Comparison is case-insensitive.
419+
func (q *QuietHoursConfig) IsBypassed(login string) bool {
420+
return containsUsername(q.BypassUsers, login)
421+
}
422+
423+
func containsUsername(users []string, login string) bool {
416424
lower := strings.ToLower(login)
417-
for _, u := range c.ThrottleBypassUsers {
418-
if strings.ToLower(u) == lower {
425+
for _, user := range users {
426+
if strings.ToLower(user) == lower {
419427
return true
420428
}
421429
}
422430
return false
423431
}
424432

433+
// IsThrottleBypassed reports whether the given GitHub login is in
434+
// the ThrottleBypassUsers list. Comparison is case-insensitive.
435+
func (c *CIConfig) IsThrottleBypassed(login string) bool {
436+
return containsUsername(c.ThrottleBypassUsers, login)
437+
}
438+
425439
// ResolvedMaxRepos returns the maximum number of repos to poll.
426440
// Defaults to 100 if not set or non-positive.
427441
func (c *CIConfig) ResolvedMaxRepos() int {

internal/config/quiet_hours_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,13 @@ func TestQuietHoursWindowActive(t *testing.T) {
180180
})
181181
}
182182

183+
func TestQuietHoursBypassUsers(t *testing.T) {
184+
q := QuietHoursConfig{BypassUsers: []string{"Trusted-User"}}
185+
assert.True(t, q.IsBypassed("trusted-user"))
186+
assert.True(t, q.IsBypassed("TRUSTED-USER"))
187+
assert.False(t, q.IsBypassed("other-user"))
188+
}
189+
183190
func TestQuietHoursConfigTOMLParsing(t *testing.T) {
184191
configPath := filepath.Join(t.TempDir(), "config.toml")
185192
err := os.WriteFile(configPath, []byte(`
@@ -191,6 +198,7 @@ start = "23:00"
191198
end = "05:00"
192199
timezone = "US/Central"
193200
throttle_interval = "90m"
201+
bypass_users = ["trusted-user"]
194202
`), 0o644)
195203
require.NoError(t, err)
196204

@@ -201,4 +209,5 @@ throttle_interval = "90m"
201209
assert.Equal(t, "05:00", q.End)
202210
assert.Equal(t, "US/Central", q.Timezone)
203211
assert.Equal(t, "90m", q.ThrottleInterval)
212+
assert.Equal(t, []string{"trusted-user"}, q.BypassUsers)
204213
}

internal/daemon/ci_poller.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -605,18 +605,20 @@ type throttleDecision struct {
605605
}
606606

607607
// throttlePR reports whether the PR was reviewed recently enough to defer this
608-
// push. Bypass users skip the base throttle interval, but the quiet-hours
609-
// interval applies to everyone while the window is active. The throttle is
610-
// purely time-based on the most recent panel run for the PR (any HEAD SHA).
611-
// Pure decision, no side effects: the caller publishes the deferred status
612-
// via postDeferredStatus after its retention/supersede bookkeeping.
608+
// push. Base-throttle bypass users skip the ordinary interval, while
609+
// quiet-hours bypass users skip only the additional interval active during
610+
// that window. The throttle is purely time-based on the most recent panel run
611+
// for the PR (any HEAD SHA). Pure decision, no side effects: the caller
612+
// publishes the deferred status via postDeferredStatus after its
613+
// retention/supersede bookkeeping.
613614
func (p *CIPoller) throttlePR(ghRepo string, pr ghPR, cfg *config.Config) (throttleDecision, error) {
614615
base := cfg.CI.ResolvedThrottleInterval()
615616
if cfg.CI.IsThrottleBypassed(pr.Author.Login) {
616617
base = 0
617618
}
618619
throttle := base
619-
if q := p.quietHours; q != nil && q.Active(p.nowFn()) && q.Interval > throttle {
620+
if q := p.quietHours; q != nil && q.Active(p.nowFn()) &&
621+
!cfg.CI.QuietHours.IsBypassed(pr.Author.Login) && q.Interval > throttle {
620622
throttle = q.Interval
621623
}
622624
if throttle <= 0 {
@@ -634,7 +636,7 @@ func (p *CIPoller) throttlePR(ghRepo string, pr ghPR, cfg *config.Config) (throt
634636
throttled: true,
635637
quietOnly: base <= 0 || elapsed >= base,
636638
// With the quiet-hours interval this can overstate the wait for
637-
// bypass users, who become eligible at the first poll after the
639+
// base-throttle bypass users, who become eligible at the first poll after the
638640
// window ends. The status is advisory; capping at the window end
639641
// isn't worth the wrap-around complexity.
640642
nextEligible: lastReview.Add(throttle),

internal/daemon/ci_poller_quiet_hours_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,56 @@ func TestCIPollerProcessPR_QuietHoursThrottlesBypassUser(t *testing.T) {
114114
"member must not be canceled")
115115
}
116116

117+
func TestCIPollerProcessPR_QuietHoursBypassSkipsQuietThrottle(t *testing.T) {
118+
now := time.Now()
119+
h := newQuietHoursHarness(t, "1h", []string{"trusted-user"}, now)
120+
h.Cfg.CI.QuietHours.BypassUsers = []string{"TRUSTED-USER"}
121+
h.Poller.quietHours = quietWindow(t, now, "1h", true)
122+
123+
pr := func(sha string) ghPR {
124+
return ghPR{
125+
Number: 98, HeadRefOid: sha, BaseRefName: "main",
126+
Author: ghPRAuthor{Login: "trusted-user"},
127+
}
128+
}
129+
130+
err := h.Poller.processPR(context.Background(), "acme/api", pr("first-sha"), h.Cfg)
131+
require.NoError(t, err, "first processPR")
132+
require.True(t, h.hasPanel(t, "acme/api", 98, "first-sha"))
133+
134+
err = h.Poller.processPR(context.Background(), "acme/api", pr("second-sha"), h.Cfg)
135+
require.NoError(t, err, "second processPR")
136+
assert.True(t, h.hasPanel(t, "acme/api", 98, "second-sha"),
137+
"author bypassing both throttle layers should be reviewed immediately")
138+
}
139+
140+
func TestCIPollerProcessPR_QuietHoursBypassPreservesBaseThrottle(t *testing.T) {
141+
now := time.Now()
142+
h := newQuietHoursHarness(t, "1h", nil, now)
143+
h.Cfg.CI.QuietHours.BypassUsers = []string{"TRUSTED-USER"}
144+
h.Poller.quietHours = quietWindow(t, now, "1h", true)
145+
146+
pr := func(sha string) ghPR {
147+
return ghPR{
148+
Number: 99, HeadRefOid: sha, BaseRefName: "main",
149+
Author: ghPRAuthor{Login: "trusted-user"},
150+
}
151+
}
152+
153+
err := h.Poller.processPR(context.Background(), "acme/api", pr("first-sha"), h.Cfg)
154+
require.NoError(t, err, "first processPR")
155+
require.True(t, h.hasPanel(t, "acme/api", 99, "first-sha"))
156+
157+
err = h.Poller.processPR(context.Background(), "acme/api", pr("second-sha"), h.Cfg)
158+
require.NoError(t, err, "second processPR")
159+
assert.False(t, h.hasPanel(t, "acme/api", 99, "second-sha"),
160+
"quiet-hours bypass must not bypass the ordinary throttle")
161+
162+
active, err := h.DB.GetActivePanelsForPR("acme/api", 99)
163+
require.NoError(t, err)
164+
assert.Empty(t, active, "ordinary throttle must supersede the stale panel")
165+
}
166+
117167
func TestCIPollerProcessPR_QuietHoursElapsedBaseKeepsPanel(t *testing.T) {
118168
assert := assert.New(t)
119169
// A non-bypass contributor whose base throttle (1h) has elapsed but whose

0 commit comments

Comments
 (0)