Skip to main content
← Back to list
01Issue
FeatureOpenExtensions

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

  1. Reviewer submits an APPROVED review on a PR.
  2. Same reviewer later adds a COMMENTED review (e.g., a passing thought, no actionable feedback).
  3. fetchPrReviews returns that reviewer with state: "COMMENTED", even though GitHub still considers the PR approved by them.
  4. Downstream consumers (the fix method, 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

02Bog Flow
OPENTRIAGEDIN PROGRESSSHIPPED

Open

4/7/2026, 11:29:25 PM

No activity in this phase yet.

03Sludge Pulse

Sign in to post a ripple.