[PR #339] fix(detect-unreviewed): require human approval #339

New issue

Open

opened 2026-10-26 22:38:41 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:38:41 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/339
Author: @christian-byrne
Created: 10/2/2026
Status: 🔄 Open

Base: main ← Head: fix/detect-unreviewed-human-approvals

📝 Commits (2)

a782f18 fix(detect-unreviewed): require human approval
a4d905f fix(detect-unreviewed): fail closed on review identity

📊 Changes

4 files changed (+223 additions, -5 deletions)

View changed files

➕ .github/detect-unreviewed-merge/tests/approval.test.cjs (+177 -0)
📝 .github/workflows/detect-unreviewed-merge.yml (+10 -4)
➕ .github/workflows/test-detect-unreviewed-merge.yml (+30 -0)
📝 docs/callers/detect-unreviewed-merge.md (+6 -1)

📄 Description

Summary

exclude GitHub accounts with type: Bot from approval evidence in both detector modes
preserve human approvals and all bot review automation
add a hermetic regression harness that executes the reusable workflow's actual inline script

Reproduction

ComfyUI_frontend #19867 had no human APPROVED review. Its only APPROVED review was from coderabbitai[bot], but Detect Unreviewed Merge run 37010691881 logged PR has an approving review — no action needed, so no tracker issue was created.

Root cause: both any-approval and latest-per-reviewer tested review state without checking review.user.type. The detector therefore treated bot approval as SOC 2 peer-review evidence.

Acceptance criteria

a current human APPROVED review suppresses tracker creation
a bot-only APPROVED review creates the same tracker record as no approval
both latest-per-reviewer and any-approval obey the human-only rule
bot reviews remain enabled and unchanged; branch protection is unchanged
consumer pins move through the existing caller-bump workflow after merge

Validation

node --test .github/detect-unreviewed-merge/tests/*.test.cjs (4 passed)
python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py' -v (502 passed, 29 skipped)
python3 .github/workflow-pins/check_workflow_pins.py
git diff --check

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/339 **Author:** [@christian-byrne](https://github.com/christian-byrne) **Created:** 10/2/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/detect-unreviewed-human-approvals` --- ### 📝 Commits (2) - [`a782f18`](https://github.com/Comfy-Org/github-workflows/commit/a782f1805e9759359fe9b095e744c64850b56e57) fix(detect-unreviewed): require human approval - [`a4d905f`](https://github.com/Comfy-Org/github-workflows/commit/a4d905f542052228d03342a045b89df589ceb478) fix(detect-unreviewed): fail closed on review identity ### 📊 Changes **4 files changed** (+223 additions, -5 deletions) <details> <summary>View changed files</summary> ➕ `.github/detect-unreviewed-merge/tests/approval.test.cjs` (+177 -0) 📝 `.github/workflows/detect-unreviewed-merge.yml` (+10 -4) ➕ `.github/workflows/test-detect-unreviewed-merge.yml` (+30 -0) 📝 `docs/callers/detect-unreviewed-merge.md` (+6 -1) </details> ### 📄 Description ## Summary - exclude GitHub accounts with `type: Bot` from approval evidence in both detector modes - preserve human approvals and all bot review automation - add a hermetic regression harness that executes the reusable workflow's actual inline script ## Reproduction [ComfyUI_frontend #19867](https://github.com/Comfy-Org/ComfyUI_frontend/pull/19867) had no human `APPROVED` review. Its only `APPROVED` review was from `coderabbitai[bot]`, but [Detect Unreviewed Merge run 37010691881](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/37010691881) logged `PR has an approving review — no action needed`, so no tracker issue was created. Root cause: both `any-approval` and `latest-per-reviewer` tested review state without checking `review.user.type`. The detector therefore treated bot approval as SOC 2 peer-review evidence. ## Acceptance criteria - a current human `APPROVED` review suppresses tracker creation - a bot-only `APPROVED` review creates the same tracker record as no approval - both `latest-per-reviewer` and `any-approval` obey the human-only rule - bot reviews remain enabled and unchanged; branch protection is unchanged - consumer pins move through the existing caller-bump workflow after merge ## Validation - `node --test .github/detect-unreviewed-merge/tests/*.test.cjs` (4 passed) - `python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py' -v` (502 passed, 29 skipped) - `python3 .github/workflow-pins/check_workflow_pins.py` - `git diff --check` --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-26 22:38:41 +00:00