Skip redundant app installation churn for unchanged delta

In delta mode, when an app is present in both the previous and current
suborg/repo config:
- If suborg targeting is unchanged, skip the app entirely (no selection or
  unselection) — avoids re-adding all suborg repos on unrelated config edits
- If targeting changed, emit only the diff (newly targeted repos to add,
  no-longer targeted repos to remove) instead of re-adding the full set
- Repo-level: only select when the app is newly added to the repo config

Add unit tests covering the skip, targeting-diff, and org-'all' precedence
cases for _buildAppChangesFromDelta.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: decyjphr

lib/settings.js

@@ -1594,34 +1594,37 @@ class Settings {
       const currentRepos = await resolveSuborgRepos(currentConfig)
       const previousRepos = await resolveSuborgRepos(previousConfig)
 
-      // Apps present in current config: repos to add = currentRepos
+      // App newly added to this suborg: select all currently targeted repos
       for (const slug of currentAppSlugs) {
+        if (previousAppSlugs.has(slug)) continue
         const entry = ensureEntry(slug)
         if (!entry) continue
         for (const repo of currentRepos) {
           entry.repository_selection.add(repo)
         }
       }
 
-      // Apps removed from config: repos to unselect = previousRepos
+      // App removed from this suborg: unselect all previously targeted repos
       for (const slug of previousAppSlugs) {
-        if (currentAppSlugs.has(slug)) continue // still present
+        if (currentAppSlugs.has(slug)) continue
         const entry = ensureEntry(slug)
         if (!entry) continue
         for (const repo of previousRepos) {
           entry.repository_unselection.add(repo)
         }
       }
 
-      // Apps still present but targeting changed: unselect repos no longer targeted
-      for (const slug of currentAppSlugs) {
-        if (!previousAppSlugs.has(slug)) continue // newly added, no unselection needed
-        const entry = ensureEntry(slug)
-        if (!entry) continue
-        for (const repo of previousRepos) {
-          if (!currentRepos.has(repo)) {
-            entry.repository_unselection.add(repo)
-          }
+      // App present in both: only act on the targeting diff. If the targeting
+      // is unchanged, skip entirely to avoid redundant churn.
+      const addedRepos = [...currentRepos].filter(r => !previousRepos.has(r))
+      const removedRepos = [...previousRepos].filter(r => !currentRepos.has(r))
+      if (addedRepos.length > 0 || removedRepos.length > 0) {
+        for (const slug of currentAppSlugs) {
+          if (!previousAppSlugs.has(slug)) continue // handled as "newly added" above
+          const entry = ensureEntry(slug)
+          if (!entry) continue
+          for (const repo of addedRepos) entry.repository_selection.add(repo)
+          for (const repo of removedRepos) entry.repository_unselection.add(repo)
         }
       }
     }
@@ -1646,14 +1649,17 @@ class Settings {
       }
       const previousAppSlugs = new Set(previousApps.map(a => a.app_slug).filter(Boolean))
 
-      // Apps present in current config: add this repo
+      // App newly added to this repo config: select this repo. If the app was
+      // already present in the previous version, its selection is unchanged —
+      // skip to avoid redundant churn.
       for (const slug of currentAppSlugs) {
+        if (previousAppSlugs.has(slug)) continue
         const entry = ensureEntry(slug)
         if (!entry) continue
         entry.repository_selection.add(repo.repo)
       }
 
-      // Apps removed from config: unselect this repo
+      // App removed from this repo config: unselect this repo
       for (const slug of previousAppSlugs) {
         if (currentAppSlugs.has(slug)) continue
         const entry = ensureEntry(slug)

test/unit/lib/settings.test.js

@@ -1615,4 +1615,106 @@ repository:
       expect(result).toEqual({ repos: [], previousPluginSections: [] })
     })
   })
+
+  describe('_buildAppChangesFromDelta', () => {
+    let settings
+    const AppOctokitClient = require('../../../lib/appOctokitClient')
+    const RepoSelector = require('../../../lib/repoSelector')
+
+    beforeEach(() => {
+      stubConfig = { restrictedRepos: {} }
+      settings = createSettings(stubConfig)
+      // Map app slug -> installation id
+      jest.spyOn(AppOctokitClient.prototype, 'listOrgInstallations').mockResolvedValue([
+        { app_slug: 'my-app', id: 42 }
+      ])
+    })
+
+    afterEach(() => {
+      jest.restoreAllMocks()
+    })
+
+    it('skips an app when suborg targeting and app_installations are unchanged', async () => {
+      // Same targeting resolves to the same repos in both versions
+      jest.spyOn(RepoSelector.prototype, 'resolve').mockResolvedValue(new Set(['repo-a', 'repo-b']))
+
+      // Current suborg config: app present
+      settings.subOrgConfigs = {
+        frontend: {
+          suborgrepos: ['repo-a', 'repo-b'],
+          app_installations: [{ app_slug: 'my-app' }]
+        }
+      }
+      // Previous version (baseRef): identical app_installations
+      settings.loadYamlFromRef = jest.fn().mockResolvedValue({
+        suborgrepos: ['repo-a', 'repo-b'],
+        app_installations: [{ app_slug: 'my-app' }]
+      })
+
+      const result = await settings._buildAppChangesFromDelta(
+        settings.github,
+        'my-enterprise',
+        [{ repo: 'frontend', path: '.github/suborgs/frontend.yml' }],
+        [],
+        'prev-sha'
+      )
+
+      // No churn: nothing to add or remove
+      expect(result).toEqual([])
+    })
+
+    it('emits only the targeting diff when suborg repos change', async () => {
+      // previous: repo-a, repo-b ; current: repo-b, repo-c
+      jest.spyOn(RepoSelector.prototype, 'resolve')
+        .mockResolvedValueOnce(new Set(['repo-b', 'repo-c'])) // current
+        .mockResolvedValueOnce(new Set(['repo-a', 'repo-b'])) // previous
+
+      settings.subOrgConfigs = {
+        frontend: {
+          suborgrepos: ['repo-b', 'repo-c'],
+          app_installations: [{ app_slug: 'my-app' }]
+        }
+      }
+      settings.loadYamlFromRef = jest.fn().mockResolvedValue({
+        suborgrepos: ['repo-a', 'repo-b'],
+        app_installations: [{ app_slug: 'my-app' }]
+      })
+
+      const result = await settings._buildAppChangesFromDelta(
+        settings.github,
+        'my-enterprise',
+        [{ repo: 'frontend', path: '.github/suborgs/frontend.yml' }],
+        [],
+        'prev-sha'
+      )
+
+      expect(result).toHaveLength(1)
+      expect(result[0].app_slug).toBe('my-app')
+      expect(result[0].repository_selection.sort()).toEqual(['repo-c'])
+      expect(result[0].repository_unselection.sort()).toEqual(['repo-a'])
+    })
+
+    it('skips apps configured as repository_selection: all at org level', async () => {
+      jest.spyOn(RepoSelector.prototype, 'resolve').mockResolvedValue(new Set(['repo-a']))
+
+      settings.config = {
+        ...settings.config,
+        app_installations: [{ app_slug: 'my-app', repository_selection: 'all' }]
+      }
+      settings.subOrgConfigs = {
+        frontend: { suborgrepos: ['repo-a'], app_installations: [{ app_slug: 'my-app' }] }
+      }
+      settings.loadYamlFromRef = jest.fn().mockResolvedValue({})
+
+      const result = await settings._buildAppChangesFromDelta(
+        settings.github,
+        'my-enterprise',
+        [{ repo: 'frontend', path: '.github/suborgs/frontend.yml' }],
+        [],
+        'prev-sha'
+      )
+
+      expect(result).toEqual([])
+    })
+  })
 }) // Settings Tests