fix(rulesets): resolve {{EXTERNALLY_DEFINED}} before diffing so unchanged rulesets don't plan updates

lib/plugins/diffable.js

@@ -66,6 +66,11 @@ module.exports = class Diffable extends ErrorStash {
       let filteredEntries = this.filterEntries()
       // this.log.debug(`filtered entries are ${JSON.stringify(filteredEntries)}`)
       return this.find().then(existingRecords => {
+        // Let plugins resolve config placeholders (e.g. {{EXTERNALLY_DEFINED}}) against
+        // the live records before any comparison, so placeholders never report changes.
+        if (typeof this.resolveOverrides === 'function') {
+          filteredEntries = this.resolveOverrides(existingRecords, filteredEntries)
+        }
         this.log.debug(` ${JSON.stringify(existingRecords, null, 2)} \n\n ${JSON.stringify(filteredEntries, null, 2)} `)
 
         const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields)

lib/plugins/rulesets.js

@@ -4,11 +4,11 @@ const MergeDeep = require('../mergeDeep')
 const Overrides = require('./overrides')
 const ignorableFields = []
 const overrides = {
-  'required_status_checks': {
-    'action': 'delete',
-    'parents': 3,
-    'type': 'array'
-  },
+  required_status_checks: {
+    action: 'delete',
+    parents: 3,
+    type: 'array'
+  }
 }
 
 const version = {
@@ -94,6 +94,17 @@ module.exports = class Rulesets extends Diffable {
     return existing.name === attrs.name
   }
 
+  // Resolve {{EXTERNALLY_DEFINED}} placeholders against the matching live ruleset
+  // before diffing, mirroring how branches.js resolves overrides inside its
+  // comparison. Without this the comparison sees the literal placeholder string,
+  // so every plan reports an update for rulesets that use overrides.
+  resolveOverrides (existingRecords, entries) {
+    return entries.map(attrs => {
+      const existing = existingRecords.find(record => this.comparator(record, attrs))
+      return Overrides.removeOverrides(overrides, structuredClone(attrs), existing || {})
+    })
+  }
+
   changed (existing, attrs) {
     const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields)
     const merged = mergeDeep.compareDeep(existing, attrs)

test/unit/lib/plugins/rulesets.test.js

@@ -188,7 +188,7 @@ describe('Rulesets', () => {
   })
 
   describe('when {{EXTERNALLY_DEFINED}} is present in "required_status_checks" and status checks exist in GitHub', () => {
-    it('it retains the status checks from GitHub and everything else is reset to the safe-settings', () => {
+    it('skips the placeholder-only ruleset and updates the genuinely changed ones', () => {
       // Mock the GitHub API response
       github.paginate = jest.fn().mockResolvedValue([
         generateRequestRuleset(
@@ -251,22 +251,12 @@ describe('Rulesets', () => {
       )
 
       return plugin.sync().then(() => {
+        // Ruleset 1 only differs by the {{EXTERNALLY_DEFINED}} placeholder, which
+        // resolves to the live status checks, so it must not be updated at all.
+        expect(github.request).toHaveBeenCalledTimes(2)
         expect(github.request).toHaveBeenNthCalledWith(
           1,
           'PUT /repos/{owner}/{repo}/rulesets/{id}',
-          generateResponseRuleset(
-            1,
-            'All branches 1',
-            repo_conditions,
-            [
-              { context: 'Custom Check 1' },
-              { context: 'Custom Check 2' }
-            ]
-          )
-        )
-        expect(github.request).toHaveBeenNthCalledWith(
-          2,
-          'PUT /repos/{owner}/{repo}/rulesets/{id}',
           generateResponseRuleset(
             2,
             'All branches 2',
@@ -278,7 +268,7 @@ describe('Rulesets', () => {
           )
         )
         expect(github.request).toHaveBeenNthCalledWith(
-          3,
+          2,
           'PUT /repos/{owner}/{repo}/rulesets/{id}',
           generateResponseRuleset(
             3,
@@ -369,7 +359,7 @@ describe('Rulesets', () => {
   })
 
   describe('[org] when {{EXTERNALLY_DEFINED}} is present in "required_status_checks" and status checks exist in GitHub', () => {
-    it('it retains the status checks from GitHub', () => {
+    it('reports no changes when only the placeholder differs from GitHub', () => {
       // Mock the GitHub API response
       github.paginate = jest.fn().mockResolvedValue([
         generateRequestRuleset(
@@ -402,20 +392,52 @@ describe('Rulesets', () => {
       )
 
       return plugin.sync().then(() => {
-        expect(github.request).toHaveBeenNthCalledWith(
-          1,
-          'PUT /orgs/{org}/rulesets/{id}',
-          generateResponseRuleset(
-            1,
-            'All branches 1',
-            org_conditions,
-            [
-              { context: 'Custom Check 1' },
-              { context: 'Custom Check 2' }
-            ],
-            true
-          )
-        )
+        // The placeholder resolves to the live status checks, so the ruleset is unchanged
+        expect(github.request).not.toHaveBeenCalled()
+      })
+    })
+  })
+
+  describe('in nop mode', () => {
+    function configureNop (config) {
+      return new Rulesets(true, github, { owner: 'jitran', repo: 'test' }, config, log, [], 'repo')
+    }
+
+    beforeEach(() => {
+      github.request.endpoint = Object.assign(
+        jest.fn().mockImplementation((route, parms) => { return { url: route, body: parms } }),
+        { merge: github.request.endpoint.merge }
+      )
+    })
+
+    it('does not plan an update when {{EXTERNALLY_DEFINED}} matches the status checks in GitHub', () => {
+      github.paginate = jest.fn().mockResolvedValue([
+        generateRequestRuleset(1, 'All branches 1', repo_conditions, [{ context: 'Custom Check 1' }])
+      ])
+
+      const plugin = configureNop([
+        generateRequestRuleset(1, 'All branches 1', repo_conditions, [{ context: '{{EXTERNALLY_DEFINED}}' }])
+      ])
+
+      return plugin.sync().then(res => {
+        // sync resolves with nothing when no changes are detected
+        const messages = (res || []).flat(2).map(nopCommand => nopCommand.action?.msg)
+        expect(messages).not.toContain('Update Ruleset')
+      })
+    })
+
+    it('still plans an update when the config genuinely differs from GitHub', () => {
+      github.paginate = jest.fn().mockResolvedValue([
+        generateRequestRuleset(1, 'All branches 1', repo_conditions, [{ context: 'Custom Check 1' }])
+      ])
+
+      const plugin = configureNop([
+        generateRequestRuleset(1, 'All branches 1', repo_conditions, [{ context: 'Other Check' }])
+      ])
+
+      return plugin.sync().then(res => {
+        const messages = res.flat(2).map(nopCommand => nopCommand.action?.msg)
+        expect(messages).toContain('Update Ruleset')
       })
     })
   })