Skip to content

fix(ci): preserve review states across empty and merge commits without game changes - #4402

Merged
LucasHT22 merged 5 commits into
hackclub:mainfrom
SSoggyTacoMan:fix/ci-merge-and-empty-commit-handling
Oct 20, 2026
Merged

LucasHT22 merged 5 commits into
hackclub:mainfrom
SSoggyTacoMan:fix/ci-merge-and-empty-commit-handling

Conversation

@SSoggyTacoMan

Copy link
Copy Markdown
Contributor

Summary

When a maintainer updates a PR branch with main or an empty commit is pushed without touching submission game files:

  1. Review state was lost or bypassed: In review-state.mjs, CHANGES_REQUESTED and APPROVED strictly matched r.commit_id === headSha. A merge commit changed headSha, which falsely cleared CHANGES_REQUESTED back to Ready for Playtest (as seen on [Sprig App] Galaxy shooter #4046) or demoted an APPROVED PR back to Ready for Playtest.
  2. Stale timer corruption: In stale-review-queue.mjs, author commits did not count towards author activity (only issue comments did), and handleUntouchedSubmission used pullRequest.updated_at (which maintainer branch updates refreshed).

Changes

  • Diff-aware review reconciliation: Added isReviewCommitOutdated in review-utils.mjs comparing the diff between review.commit_id and headSha for submission files in games/.
  • Review state preservation: Updated reconcileReviewStatus in review-state.mjs to accept an outdated check, preserving changes_requested and approved states across commits that do not touch game files.
  • Triage and review sync integration: Wired auto-triage.mjs and sync-reviews.mjs into isReviewCommitOutdated.
  • Author activity tracking: Updated stale-review-queue.mjs to count author commits toward activity and ignore automated/maintainer updates when computing untouched submission age.
  • Unit tests: Added unit tests verifying review status preservation across non-game commits and isReviewCommitOutdated compare diff checking.

@vercel

vercel Bot commented Oct 16, 2026

Copy link
Copy Markdown

@SSoggyTacoMan is attempting to deploy a commit to the Hack Club Team on Vercel.

A member of the Team first needs to authorize it.

@LucasHT22 LucasHT22 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s an edge case with cherry picked commits but lgtm for now

@LucasHT22
LucasHT22 added this pull request to the merge queue Oct 20, 2026
Merged via the queue into hackclub:main with commit d396360 Oct 20, 2026
6 of 7 checks passed
@SSoggyTacoMan
SSoggyTacoMan deleted the fix/ci-merge-and-empty-commit-handling branch October 20, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants