Safe Settings do not update org Branch Protection Rulesets after created #998

Activity

1. ddhananjay

   added

   bugSomething isn't workingSomething isn't working

   on Jun 15, 2026
2. decyjphr commented on Jun 17, 2026

   

   decyjphr

   on Jun 17, 2026

   Collaborator

   More actions

   The issue is mergeDeep had a bug. Here are the details:

   The Bug

   When you manually removed required_reviewers from a ruleset via the GitHub UI, safe-settings failed to detect the change because the comparison logic in mergeDeep.js was silently ignoring array items without identifying properties (like name, username, etc.).

   What was happening:

   • GitHub state: required_reviewers: [] (empty after manual removal)
   • YAML config: required_reviewers: [{minimum_approvals: 1, file_patterns: ["*.js"], reviewer: {...}}]
   • Result: Comparison returned hasChanges: false ❌

   Root Cause

   In mergeDeep.js, the processArrays() method tracks array items using identifying properties. The required_reviewers objects don't have these properties, so they were never added to the visited object and therefore never compared.

   The Fix

   I modified mergeDeep.js in three places to use JSON.stringify as a fallback identifier when objects lack identifying properties:

   1. In processArrays() (line 206): Changed from silently skipping items to using JSON representation as a unique key
   2. In array comparison filters (lines 225-245): Updated both additions and deletions filters to use the fallback identifier
   3. In compareDeepIfVisited() (line 259): Updated to use the same fallback identifier for consistency

   Verification

   ✅ Created test case: "detects when required_reviewers array changes from populated to empty"
   ✅ Manual testing confirms the fix: comparison now returns hasChanges: true
   ✅ StandardJS linting passed
   ✅ Syntax validation passed

   Files Modified

   • mergeDeep.js - Fixed comparison logic
   • rulesets.test.js - Added regression test

   The fix is general-purpose and will now work for ANY array field where items lack identifying properties, making drift detection more robust across the board.
3. decyjphr commented on Jun 17, 2026

   

   decyjphr

   on Jun 17, 2026

   Collaborator

   More actions

   @ddhananjay this has been fixed in yadhav/fix-recent-issues branch