issue-lifecycle: COMMENTED PR review can overwrite a prior decisive state in fetchPrReviews
Opened by stack72 · 4/7/2026· GitHub #53
Background
issue-lifecycle/extensions/models/_lib/issue_tracker.ts fetchPrReviews keys reviews by user.login in a Map, so the latest review per reviewer wins. This is intentional and matches GitHub's own PR UI semantics — a reviewer who first requested changes and then approved is considered to have approved, and we deliberately do not surface withdrawn feedback into the lifecycle's fix loop.
The narrow gap
There is one case where latest-wins is wrong: a COMMENTED review submitted after a decisive state. In GitHub's own UI a COMMENTED review does not dismiss a prior APPROVED or CHANGES_REQUESTED — the PR retains its decisive state. Our current code overwrites it.
Reproduction
- Reviewer submits an
APPROVEDreview on a PR. - Same reviewer later adds a
COMMENTEDreview (e.g., a passing thought, no actionable feedback). fetchPrReviewsreturns that reviewer withstate: "COMMENTED", even though GitHub still considers the PR approved by them.- Downstream consumers (the
fixmethod, the swamp.club lifecycle entries) see a less-decisive state than reality.
Suggested fix
In the loop that ingests reviews, only overwrite the existing entry if the incoming review has a decisive state, OR keep a state-priority ordering (CHANGES_REQUESTED > APPROVED > COMMENTED > PENDING) and only upgrade, never downgrade.
const PRIORITY: Record<string, number> = {
CHANGES_REQUESTED: 3,
APPROVED: 2,
COMMENTED: 1,
PENDING: 0,
};
for (const review of reviewsRaw) {
const existing = reviewMap.get(review.user.login);
if (existing && PRIORITY[review.state] <= PRIORITY[existing.state]) {
// Don't downgrade — preserve the more decisive earlier state.
continue;
}
reviewMap.set(review.user.login, { state: review.state, body: review.body, comments: [] });
}Inline comments accumulated separately are unaffected — they're keyed by reviewer in a second pass and merged in.
Why this is filed instead of fixed inline
This was surfaced by an adversarial review on the CI-fix PR for issue-lifecycle. The reviewer's original framing ("loses earlier reviews from the same user") was too broad — the chronological/latest-wins behavior is intentional and correct in the common case. Filing this so the narrow real fix gets a thoughtful patch instead of being rushed into a CI PR.
Automoved by swampadmin from GitHub issue #53
Open
No activity in this phase yet.
Sign in to post a ripple.