Skip to content

Fail closed for draft PR gate skips - #48

Open
manuelsampedro1 wants to merge 1 commit into
permission-protocol:mainfrom
manuelsampedro1:codex/fail-closed-draft-prs
Open

Fail closed for draft PR gate skips#48
manuelsampedro1 wants to merge 1 commit into
permission-protocol:mainfrom
manuelsampedro1:codex/fail-closed-draft-prs

Conversation

@manuelsampedro1

Copy link
Copy Markdown

Summary

  • fail closed by default when a PR is still draft instead of setting approved=true without receipt verification
  • add allow-draft-skip: true as an explicit opt-in for repos that still want the previous skip behavior
  • update recommended pull_request triggers to include ready_for_review

Why

Fixes #45.
Related to #36.

The previous draft skip path exited 0 before calling /api/v1/receipts/verify. With the documented/default pull_request trigger, GitHub does not rerun on ready_for_review, so a required check could stay green on the same head SHA without any receipt ever being created or verified.

Validation

  • ruby -ryaml -e 'YAML.load_file("action.yml"); puts "action.yml ok"'
  • extracted the embedded action script, substituted GitHub expressions, and ran bash -n
  • git diff --check
  • simulated draft PR branch locally:
    • default allow-draft-skip=false exits 1 with PP_DRAFT_PR_BLOCKED
    • allow-draft-skip=true preserves DRAFT_SKIPPED with a warning

@AAH20 AAH20 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Solid fix. The current main sets approved=true for draft PRs and exits before calling /api/v1/receipts/verify, so a required status check can stay green on the same head SHA after ready_for_review — exactly the gap described in #45.

Code observations:

  1. ready_for_review trigger addition — correct and necessary. Without it the default pull_request trigger types (opened, synchronize, reopened) never refire the check when a draft PR is marked ready, so the skip result persists.

  2. PP_ALLOW_DRAFT_SKIP opt-in — good UX; repos that legitimately want the old behavior can set allow-draft-skip: true, and the ::warning nudges them to keep ready_for_review in the trigger list.

  3. All outputs are set on both paths — the DRAFT_BLOCKED branch correctly sets every output (approved, receipt-id, decision, error-code, error-message, request-id, approval-url) before exit 1, matching the pattern used by every other failure branch. No downstream step can hit an undefined output.

  4. Minor: ::error guidance message could mention the allow-draft-skip: true escape hatch explicitly, so the first thing a user sees in CI is both the why and the how-to-opt-out. Not blocking — the INSTALL.md section covers it.

Approving as-is. Nice contribution.

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.

Bypass: draft PR skip can leave Deploy Gate green after ready_for_review without receipt verification

2 participants