Skip to content

feat: implement issue #539 — Compliance: ruleset-drift-pr-quality-require_last_push_approval #547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
don-petry wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: implement issue #539 — Compliance: ruleset-drift-pr-quality-require_last_push_approval #547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fd6141f
feat: implement issue #539 — Compliance: ruleset-drift-pr-quality-req…
donpetry-bot Aug 18, 2026
38c34ac
Merge branch 'main' into dev-lead/issue-539-20260818-2015
don-petry Aug 18, 2026
cac2331
chore: dev-lead update (review-changes) [skip ci-relay]
donpetry-bot Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions scripts/setup-pr-quality-ruleset.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
# What this script does:
# 1. Checks whether the "pr-quality" ruleset already exists
# 2. Creates it if missing, or updates it if present so drifted parameters
# (e.g. dismiss_stale_reviews_on_push) reconverge to the codified standard
# (idempotent — safe to re-run)
# (e.g. dismiss_stale_reviews_on_push, require_last_push_approval)
# reconverge to the codified standard (idempotent — safe to re-run)
Comment on lines +13 to +14

@codeant-ai codeant-ai Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The “idempotent — safe to re-run” claim is inaccurate because the existing-ruleset path sends a replacement PUT using RULESET_PAYLOAD, which omits bypass_actors. Re-running this script can therefore remove an existing Dependabot integration bypass (or other operational bypass actors) while reporting success. Preserve existing bypass actors in the update payload, or narrow the comment so it does not promise a non-destructive rerun. [comment mismatch]

Severity Level: Major ⚠️ 
- ❌ Dependabot bypass can be removed from `pr-quality`.
- ❌ Dependabot auto-merge may be blocked afterward.
- ⚠️ Script reports successful update despite losing actors.

Use CodeAnt Skill

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** scripts/setup-pr-quality-ruleset.sh
**Line:** 13:14
**Comment:**
	*Comment Mismatch: The “idempotent — safe to re-run” claim is inaccurate because the existing-ruleset path sends a replacement `PUT` using `RULESET_PAYLOAD`, which omits `bypass_actors`. Re-running this script can therefore remove an existing Dependabot integration bypass (or other operational bypass actors) while reporting success. Preserve existing bypass actors in the update payload, or narrow the comment so it does not promise a non-destructive rerun.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@don-petry don-petry Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in scripts/setup-pr-quality-ruleset.sh: the update path now fetches the existing ruleset's bypass_actors via gh api and merges them into the PUT payload using jq (. + {bypass_actors: $bypass}), so existing Dependabot or other bypass actors are preserved on every re-run. Added a test in scripts/tests/setup-pr-quality-ruleset.test.js asserting the script fetches and re-injects bypass_actors.

#
# Prerequisites: gh (authenticated with repo admin rights)
# Usage:
Expand Down Expand Up @@ -79,7 +79,10 @@ JSON

if [[ -n "$EXISTING_ID" ]]; then
echo " ↻ '$RULESET_NAME' ruleset already exists (id: $EXISTING_ID) — updating to ensure compliance..."
printf '%s\n' "$RULESET_PAYLOAD" | gh api "repos/$REPO/rulesets/$EXISTING_ID" --method PUT --input -
# Preserve existing bypass_actors: a PUT that omits this field removes them entirely
EXISTING_BYPASS=$(gh api "repos/$REPO/rulesets/$EXISTING_ID" | jq -c '.bypass_actors // []')
UPDATE_PAYLOAD=$(printf '%s' "$RULESET_PAYLOAD" | jq --argjson bypass "$EXISTING_BYPASS" '. + {bypass_actors: $bypass}')
printf '%s\n' "$UPDATE_PAYLOAD" | gh api "repos/$REPO/rulesets/$EXISTING_ID" --method PUT --input -
echo " ✓ '$RULESET_NAME' ruleset updated successfully."
echo ""
echo "=== Done ==="
Expand Down
9 changes: 9 additions & 0 deletions scripts/tests/setup-pr-quality-ruleset.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,13 @@ describe('setup-pr-quality-ruleset.sh codified ruleset', () => {
const script = fs.readFileSync(scriptPath, 'utf8')
expect(script).toMatch(/--method PUT/)
})

it('script preserves existing bypass_actors when updating an existing ruleset', () => {
const scriptPath = path.join(__dirname, '..', 'setup-pr-quality-ruleset.sh')
const script = fs.readFileSync(scriptPath, 'utf8')
// The update path must fetch and re-inject bypass_actors so a PUT does not
// silently remove an existing Dependabot or other integration bypass.
expect(script).toMatch(/bypass_actors/)
expect(script).toMatch(/jq.*bypass_actors/)
})
})
Loading