Repository navigation
feat: add quiet-drafts input, checked against live draft state - #205
asyncawaitpromise wants to merge 3 commits into
Conversation
Callers were told to compute quiet mode from the webhook payload:
quiet: ${{ github.event.pull_request.draft }}
That value is a snapshot in the event JSON. Marking a PR ready in the
same moment as a push delivers a synchronize event still saying
draft: true, and a job rerun replays the original payload verbatim, so
the input is frozen for the life of the run. A rerun workflow on
pull_request_review reruns the existing job and inherits it, so the
check cannot un-stick itself: the PR sits with no reviewers requested
and no status comment until a fresh push or a label toggle.
The action already has the authoritative answer. InitPR does a live
PullRequests.Get and stores the full PR, including Draft, and nothing
read it.
quiet itself is left alone. A bare bool cannot distinguish "the payload
told me this is a draft" from "I never want comments", so folding live
draft state into it would both fail to rescue callers who keep the
expression and silently quiet drafts for callers who omit the input.
Instead quiet-drafts is a separate axis, defaulting false, that consults
the API on every run. Every existing workflow keeps its current meaning,
all three old behaviors stay expressible, and the fix is a one-line
migration away from an input that cannot see a state change.
|
Codeowners approval required for this PR: |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 WalkthroughWalkthroughThe action adds a Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant CodeownersPlus
participant GitHubAPI
participant PullRequest
GitHubActions->>CodeownersPlus: pass quiet-drafts
CodeownersPlus->>GitHubAPI: initialize pull request
GitHubAPI-->>CodeownersPlus: return draft state
CodeownersPlus->>PullRequest: enable quiet mode when draft
CodeownersPlus->>GitHubAPI: create comments or reviewer requests when not quiet
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The draft-aware quiet setting is wired through to the intended notification-suppression behavior without an identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
When every required team has approved but min_reviews is still short, the re-request path calls RequestReviewers directly rather than going through requestReviews, so it never saw the quiet check that lives there. A repo with min_reviews set and quiet enabled has been getting review requests it asked not to have since that path was added in the same-team re-request change (#77). quiet-drafts makes that visible: the input promises no review requests while a PR is a draft, and this path breaks the promise. Draft state is folded into config.Quiet before any of this runs, so one condition covers both the new input and a plain quiet: true. Guarding the whole block rather than the call also drops a GetCurrentlyRequested round trip that nothing can act on when quiet.
![prism-pr-review-server[bot]](https://avatars.githubusercontent.com/in/3367263?s=60&v=4){kind=small}
| // Check if we need to re-request from a satisfied team when min_reviews is not met | ||
| // Handles the case when there min_reviews is higher than the number of teams required. | ||
| if minReviewsNeeded > 0 { | ||
| if minReviewsNeeded > 0 && !a.config.Quiet { |
There was a problem hiding this comment.
Workflows already using quiet: true now also stop the min_reviews re-request, a behavior change the PR body claims does not happen.
The sandbox blocked all shell commands, so I could not scan other call sites for additional RequestReviewers paths that bypass Quiet.
Reasoning and how to verify
Adding && !a.config.Quiet changes behavior for existing quiet: true callers: the min_reviews re-request from satisfied teams now fires only when quiet is false, whereas before this PR it fired regardless. The PR body states 'quiet is unchanged' and this is not listed under Code Changes. Either document this as an intentional behavior change in the PR description and README, or revert if it is unintended. This appears to align quiet with its documented meaning ('No Review Requests'), so it may be correct but needs explicit acknowledgment.
How to verify: Run with quiet: true and a min_reviews shortfall and compare whether RequestReviewers is called before and after this change. Expected: Review requests are no longer sent in quiet mode for the min_reviews case where they previously were.
Agent prompt:
PRism finding on internal/app/app.go:424 in multimediallc/codeowners-plus#205: Workflows already using quiet: true now also stop the min_reviews re-request, a behavior change the PR body claims does not happen. Read the review comment marked <!-- prism:finding:internal/app/app.go:42:d54fdc867820 --> on that PR, decide whether it is valid, and fix it if so; otherwise explain why not.
PRism review: merge confidence 4/5Minor findings worth a look before merge.
3 lower-severity notes
Reviews (1) · reviewed 02a5a35 · 6 changed files · Reviewed by PRism Lite |
|
Codeowners approval required for this PR: |
1 participant
Summary / Background
The documented pattern is
quiet: ${{ github.event.pull_request.draft }}, so quiet mode comes from the webhook payload, fixed when the event fires. It goes stale two ways: force-pushing and marking a PR ready at once, wherecancel-in-progresscan keep thesynchronizerun and its payload still saysdraft: true; or rerunning the job, which replays the original payload. Either way the PR sits with no reviewers requested and no status comment until someone pushes again.InitPRalready fetches the PR, so the action can answer this itself. Changingquietin place felt risky though: it is a bool, so "the payload said draft" and "I never want comments" are indistinguishable, and any new meaning lands on workflows that never opted in.quiet-draftsis one way around that, happy to go a different direction if you prefer.quietis unchanged, both default false, andquiet: truestill wins.Code Changes
internal/app/app.go:QuietDraftsfield, draft check afterInitPRmain.go:-quiet-draftsflag andINPUT_QUIET-DRAFTSaction.yml:quiet-draftsinput.github/workflows/codeowners.yml: usesquiet-drafts: trueREADME.md: quiet mode section and migration noteinternal/app/app_test.go: quiet-drafts and draft state matrix