Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions internal/crq/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ func (s *Service) Pump(ctx context.Context) (PumpResult, error) {
if err != nil {
return PumpResult{}, err
}
decision := engine.DecideFire(s.global(st, now), *next, obs.eng, now, s.policy())
global := s.global(st, now)
decision := engine.DecideFire(global, *next, obs.eng, now, s.policy())
result, err := s.applyFire(ctx, *next, obs.eng, decision, now)
if err != nil {
return result, err
Expand All @@ -269,9 +270,9 @@ func (s *Service) Pump(ctx context.Context) (PumpResult, error) {
// already posted must not starve later PRs of THEIR early Codex round —
// scan a bounded number of following queued rounds for a postable
// Codex defer (each costs one observation; ETag caching keeps it cheap).
accountBlocked := global.BlockedUntil != nil && global.BlockedUntil.After(now)
if s.cfg.RateLimitCodexDegrade && decision.Verdict == engine.FireNo &&
(strings.Contains(decision.Reason, "account blocked") ||
strings.Contains(decision.Reason, "fire slot busy")) {
(accountBlocked || !global.SlotFree) {
scanned := 0
for _, r := range st.QueuedRounds(now) {
if r.Repo == next.Repo && r.PR == next.PR {
Expand Down Expand Up @@ -660,12 +661,7 @@ func (s *Service) fireRound(ctx context.Context, round Round, obs engine.Observa
// timestamp anchors the codex cutoff too, or a SHA-less answer that
// landed before this adopted fire would never bind to the round.
r.CodexCommandID = id
for _, c := range obs.CodexCommands {
if c.ID == id && !c.CreatedAt.IsZero() {
at := c.CreatedAt.UTC()
r.CodexCommandedAt = &at
}
}
r.CodexCommandedAt = codexCommandTime(obs.CodexCommands, id)
}
st.PutRound(*r)
recorded = true
Expand Down Expand Up @@ -845,6 +841,16 @@ func newestCommandID(cmds []engine.CommandSeen) int64 {
return best.ID
}

func codexCommandTime(commands []engine.CommandSeen, id int64) *time.Time {
for _, command := range commands {
if command.ID == id && !command.CreatedAt.IsZero() {
at := command.CreatedAt.UTC()
return &at
}
}
return nil
}

// fireCoReviewWait bounds a co-review wait: CodeRabbit already reviewed the head
// but a gating Codex has not yet, and crq must not post (Codex auto-reviews, or a
// command is already outstanding). Leaving the round queued with no WaitDeadline
Expand Down Expand Up @@ -888,12 +894,7 @@ func (s *Service) fireCoReviewWait(ctx context.Context, round Round, obs engine.
}
if r.CodexCommandID == 0 && codexID != 0 {
r.CodexCommandID = codexID
for _, c := range obs.CodexCommands {
if c.ID == codexID && !c.CreatedAt.IsZero() {
at := c.CreatedAt.UTC()
r.CodexCommandedAt = &at
}
}
r.CodexCommandedAt = codexCommandTime(obs.CodexCommands, codexID)
}
st.PutRound(*r)
changed = true
Expand Down
55 changes: 55 additions & 0 deletions internal/crq/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2112,6 +2112,61 @@ func TestPumpPostsCodexDeferredDuringBlockThenFiresCodeRabbit(t *testing.T) {
}
}

func TestPumpScansPastBlockedRoundWithCodexAlreadyRequested(t *testing.T) {
ctx := context.Background()
cfg := firingConfig()
cfg.RateLimitCodexDegrade = true
cfg.RequiredBots = []string{"coderabbitai[bot]", "chatgpt-codex-connector[bot]"}
cfg.CodexCommand = "@codex review"
gh := newFakeGitHub()
for _, target := range []struct {
repo string
pr int
sha string
}{
{repo: "o/first", pr: 1, sha: "111111111abcdef0"},
{repo: "o/second", pr: 2, sha: "222222222abcdef0"},
} {
pull := ghapi.Pull{State: "open"}
pull.Head.SHA = target.sha
gh.pulls[fakeKey(target.repo, target.pr)] = pull
}
store := NewMemoryStore(cfg)
svc := NewService(cfg, gh, store, nil)
now := time.Now().UTC()
svc.now = func() time.Time { return now }
if _, err := svc.Enqueue(ctx, "o/first", 1); err != nil {
t.Fatal(err)
}
if _, err := svc.Enqueue(ctx, "o/second", 2); err != nil {
t.Fatal(err)
}
blockedUntil := now.Add(30 * time.Minute)
if _, err := store.Update(ctx, func(st *State) error {
st.Account.BlockedUntil = &blockedUntil
first := st.Round("o/first", 1)
first.CodexCommandID = 700
commandedAt := now.Add(-time.Minute)
first.CodexCommandedAt = &commandedAt
st.PutRound(*first)
return nil
}); err != nil {
t.Fatal(err)
}

res, err := svc.Pump(ctx)
if err != nil {
t.Fatal(err)
}
if res.Action != "codex_fired" || res.Repo != "o/second" || res.PR != 2 {
t.Fatalf("the blocked front round must not starve a later codex round, got %#v", res)
}
st, _, _ := store.Load(ctx)
if second := st.Round("o/second", 2); second == nil || second.CodexCommandID == 0 {
t.Fatalf("the later round must record its codex command, got %#v", second)
}
}

func TestPumpAdoptsExistingCodexCommandDuringBlock(t *testing.T) {
ctx := context.Background()
cfg := firingConfig()
Expand Down
20 changes: 9 additions & 11 deletions internal/engine/fire.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ func DecideFire(g Global, r state.Round, obs Observation, now time.Time, p Polic
}
return FireDecision{Verdict: FireNo, Reason: reason}
}
reviewedHead := false
for _, review := range obs.Reviews {
if sameBot(review.Bot, p.Bot) && review.Commit != "" && strings.HasPrefix(review.Commit, obs.Head) {
reviewedHead = true
break
}
}
if !g.SlotFree {
// Codex needs no fire slot: a round parked behind another PR's
// in-flight review can start its Codex round immediately. The round
Expand All @@ -72,13 +79,6 @@ func DecideFire(g Global, r state.Round, obs Observation, now time.Time, p Polic
// CodeRabbit already reviewed — that round belongs to the dedupe
// resolution below once the slot frees (a queued round Codex answers
// clean cannot complete, so deferring it here could wedge the wait).
reviewedHead := false
for _, review := range obs.Reviews {
if sameBot(review.Bot, p.Bot) && review.Commit != "" && strings.HasPrefix(review.Commit, obs.Head) {
reviewedHead = true
break
}
}
if !reviewedHead {
if d, ok := decideCodexDeferred(r, obs, p, "fire slot busy"); ok {
return d
Expand All @@ -94,10 +94,8 @@ func DecideFire(g Global, r state.Round, obs Observation, now time.Time, p Polic
// verdicts spend CodeRabbit quota (dedupe completes, FireCodexOnly posts
// only the Codex command, a co-review wait posts nothing), so an account
// block from another PR must not delay them.
for _, review := range obs.Reviews {
if sameBot(review.Bot, p.Bot) && review.Commit != "" && strings.HasPrefix(review.Commit, obs.Head) {
return codexAwareDedupe(r, obs, p)
}
if reviewedHead {
return codexAwareDedupe(r, obs, p)
}
if g.BlockedUntil != nil && g.BlockedUntil.After(now) {
// Degrade instead of stalling: the block only gates CodeRabbit quota,
Expand Down