fix(lifecycle): stop a deduped CI nudge from starving review/merge-conflict nudges#2403
Open
abhay-codes07 wants to merge 1 commit into
Open
fix(lifecycle): stop a deduped CI nudge from starving review/merge-conflict nudges#2403abhay-codes07 wants to merge 1 commit into
abhay-codes07 wants to merge 1 commit into
Conversation
…nflict nudges ApplyPRObservation drives three independent nudge lanes (CI failure, review feedback, merge conflict), each with its own dedup key. Each lane did `return m.sendOnce(...)`, but sendOnce returned nil for both a real send and a dedup/attempt-cap no-op. So while a PR's CI stayed failing on the same commit (a normal multi-poll state), the CI lane deduped to a no-op yet still returned, and the review-feedback and merge-conflict lanes below were never reached. A reviewer's "please fix" nudge was silently suppressed for as long as CI was red. Make sendOnce report whether it actually sent (bool), and have each lane return early only on a real send or error, falling through to the next lane on a dedup no-op. This preserves the one-nudge-per-tick priority for a fresh CI failure while letting independent review/merge-conflict feedback through once the CI nudge is already delivered and deduping. Other sendOnce callers ignore the new bool. Adds a regression test covering the deduped-CI-then-new-review case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2400.
What
ApplyPRObservationdrives three independent nudge lanes for a PR (CI failure, review feedback, merge conflict), each with its own dedup key. Each lane didreturn m.sendOnce(...), butsendOncereturnednilfor both a real send and a dedup / attempt-cap no-op. So while a PR's CI stayed failing on the same commit (a normal multi-poll state), the CI lane deduped to a no-op yet still returned — and the review-feedback and merge-conflict lanes below were never reached. A reviewer's "please fix" nudge was silently suppressed for as long as CI was red.Change
Make
sendOncereport whether it actually sent ((bool, error)), and have each lane return early only on a real send or an error, falling through to the next lane on a dedup no-op. This preserves the one-nudge-per-tick priority for a fresh CI failure while letting independent review/merge-conflict feedback through once the CI nudge is already delivered and deduping. The othersendOncecallers ignore the new bool.Tests
TestPRObservation_ReviewNudgeNotStarvedByDedupedCI: CI fails (nudge feat: implement web dashboard with attention-zone UI and API routes #1), then CI unchanged + a new review comment → the review nudge must still fire. Fails before the change (only 1 nudge), passes after.go test ./internal/lifecycle/...green;go vetand gofmt clean.go test -racelocally (no 64-bit CGO toolchain on my box) — leaving that to CI.cc @harshitsinghbhandari — the independent dedup keys made me read the lanes as intended-independent; happy to adjust if the priority was meant to be strict.