fix: sync all repository-owning app installations

README.md

@@ -1040,6 +1040,10 @@ node smoke-test.js --phase 1,19
 # Repository and environment variables, including pagination and no-op convergence
 node smoke-test.js --phase 1,13,21
 
+# Full-sync NOP with real Settings, restricted to the verified test-org installation
+# Clear harness CRON; see the spawned-server configuration warning below.
+CRON= node smoke-test.js --phase 22
+
 # Mix range + interactive
 npm run smoke-test:phase -- 1-3 interactive
 node smoke-test.js --phase 1-3 --interactive
@@ -1067,8 +1071,16 @@ The smoke test runs the following phases:
 | **Phase 13** | Tests the `variables` plugin (create, update, remove variables) |
 | **Phase 19** | Tests ignored `OrganizationAdmin`/`DeployKey` IDs, order-independent NOP convergence, real bypass-mode/role-ID changes, and no redundant updates (requires Phase 1) |
 | **Phase 21** | Reads all pages of 101 repository variables and 100 environment variables, verifies unchanged NOP/apply writes nothing, and updates only the two boundary variables (requires Phases 1 and 13) |
+| **Phase 22** | Runs real full-sync NOP against the verified `GH_ORG` installation with controlled enumeration/auth and read-only, org-scoped requests. Requires only Setup; multi-installation fanout and failure isolation are covered by unit/CLI tests, not this single-org smoke |
 | **Teardown** | Shuts down safe-settings, deletes test repos, teams, custom roles, and rulesets |
 
+The harness rejects a nonempty `CRON` before authentication or setup. However,
+the spawned Probot CLI reloads `.env` and can override explicit environment values.
+This check does not guarantee that the server's CRON, webhook forwarding, or
+enterprise verification remains disabled. Phase 22's controlled installation
+boundary applies to its direct NOP call, not the spawned server; verify the
+server's effective configuration separately before running against a shared App.
+
 GitHub [limits each environment to 100 variables](https://docs.github.com/en/actions/reference/workflows-and-actions/variables#limits-for-configuration-variables),
 so Phase 21 stays within that live limit. The phase records actual API next links
 and page sizes: GitHub can return fewer than the requested 100 items per page.

docs/github-action.md

@@ -10,6 +10,22 @@ Follow the [Create the GitHub App](deploy.md#create-the-github-app) guide to cre
 ## Defining the GitHub Action Workflow
 Running a full-sync with `safe-settings` can be done via `npm run full-sync`. This requires installing Node, such as with [actions/setup-node](https://github.com/actions/setup-node) (see example below). When doing so, the appropriate environment variables must be set (see the [Environment variables](#environment-variables) document for more details).
 
+Manual and scheduled (`CRON`) full syncs process every repository-owning App
+installation sequentially, using each installation's token and account login for
+its admin repository. Enterprise installations are not repository sync targets;
+they remain available for enterprise app-management context enrichment.
+An authentication, configuration, or sync failure does not stop later installations.
+The returned `{ results, errors }` retains each returned Settings result (including
+partial results) and aggregates their errors with thrown and missing-result failures.
+An info-level summary counts installations with errors as failed. `FULL_SYNC_NOP=true`
+previews every installation and still reports failures with a nonzero CLI exit.
+No eligible installations returns `null` from `syncInstallation` and exits the CLI
+nonzero with an explicit diagnostic.
+
+`GH_ORG` does not restrict this full-sync path. Do not use a multi-installation App
+for a live single-organization test by setting `GH_ORG` alone; the smoke test's
+Phase 22 uses a controlled installation list and auth boundary instead.
+
 Installation repositories are processed in batches of up to ten, with each batch
 settling before the next starts. A repository failure does not stop later
 repositories from being processed. Failures are logged with the repository name

full-sync.js

@@ -10,6 +10,11 @@ async function performFullSync (appFn, nop) {
     const app = appFn(probot, {})
     const settings = await app.syncInstallation(nop)
 
+    if (settings === null) {
+      probot.log.error('No eligible installations found for full sync.')
+      process.exit(1)
+    }
+
     if (settings.errors && settings.errors.length > 0) {
       probot.log.error('Errors occurred during full sync.')
       process.exit(1)

index.js

@@ -63,7 +63,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
         }
         const nopcommand = new NopCommand(filename, repo, null, e, 'ERROR')
         robot.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`)
-        Settings.handleError(nop, context, repo, deploymentConfig, ref, nopcommand)
+        await Settings.handleError(nop, context, repo, deploymentConfig, ref, nopcommand)
       } else {
         throw e
       }
@@ -416,22 +416,59 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
 
   async function syncInstallation (nop = false) {
     robot.log.trace('Fetching installations')
-    const installations = await listAllInstallations()
+    const installations = (await listAllInstallations()).filter(installation => {
+      // Enterprise installations supply app-management credentials, not repos.
+      if (installation.target_type === 'Enterprise') {
+        robot.log.debug(`Skipping enterprise installation ${installation.id} for repository sync`)
+        return false
+      }
+      return true
+    })
 
-    if (installations.length > 0) {
-      const installation = installations[0]
-      const github = await robot.auth(installation.id)
-      const context = {
-        payload: {
-          installation
-        },
-        octokit: github,
-        log: robot.log,
-        repo: () => { return { repo: env.ADMIN_REPO, owner: installation.account.login } }
+    if (installations.length === 0) {
+      return null
+    }
+
+    const results = []
+    const errors = []
+    let failed = 0
+
+    for (const installation of installations) {
+      try {
+        const owner = installation.account?.login
+        if (typeof owner !== 'string' || !owner.trim()) {
+          throw new Error(`Installation ${installation.id} has no account login for repository sync`)
+        }
+        robot.log.debug(`Syncing installation ${installation.id} for ${owner}`)
+        const github = await robot.auth(installation.id)
+        const context = {
+          payload: {
+            installation
+          },
+          octokit: github,
+          log: robot.log,
+          repo: () => { return { repo: env.ADMIN_REPO, owner } }
+        }
+        const result = await syncAllSettings(nop, context)
+        if (!result) {
+          // NOP configuration errors are reported without returning Settings.
+          throw new Error(`Sync of installation ${installation.id} for ${owner} returned no result`)
+        }
+        results.push(result)
+        if (result.errors?.length) {
+          failed++
+          errors.push(...result.errors)
+          robot.log.error(`Sync of installation ${installation.id} for ${owner} reported ${result.errors.length} error(s)`)
+        }
+      } catch (e) {
+        failed++
+        robot.log.error(`Failed to sync installation ${installation.id} for ${installation.account?.login}: ${e}`)
+        errors.push(e)
       }
-      return syncAllSettings(nop, context)
     }
-    return null
+
+    robot.log.info(`Synced ${installations.length - failed} of ${installations.length} installation(s); ${failed} failed`)
+    return { results, errors }
   }
 
   robot.on('push', async context => {
@@ -1015,9 +1052,13 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
     # │ │ │ │ │ │
     # * * * * * *
     */
-    cron.schedule(process.env.CRON, () => {
+    cron.schedule(process.env.CRON, async () => {
       robot.log.debug('running a task every minute')
-      syncInstallation()
+      try {
+        await syncInstallation()
+      } catch (e) {
+        robot.log.error(`Scheduled full sync failed: ${e}`)
+      }
     })
   }
 

smoke-test.js

@@ -3539,7 +3539,87 @@ async function phase19BypassActorConvergence () {
   }
 }
 
+async function phase22InstallationFullSync (app, installationId) {
+  logPhase('Phase 22: Test-org installation full-sync NOP')
+  if (!process.env.GH_ORG || process.env.GH_ORG.toLowerCase() !== ORG.toLowerCase() || process.env.CRON) {
+    throw new Error('Phase 22 requires an explicit GH_ORG and CRON unset')
+  }
+  const { data: installation } = await app.octokit.rest.apps.getInstallation({ installation_id: installationId })
+  if (installation.id !== installationId || installation.target_type !== 'Organization' ||
+      installation.account?.login?.toLowerCase() !== ORG.toLowerCase()) {
+    throw new Error('Phase 22 installation does not match the authorized test organization')
+  }
+  const { data: appInfo } = await app.octokit.rest.apps.getAuthenticated()
+  const { data: admin } = await octokit.rest.repos.get({ owner: ORG, repo: ADMIN_REPO })
+  if (admin.owner.login.toLowerCase() !== ORG.toLowerCase()) {
+    throw new Error('Phase 22 admin repository owner does not match the test organization')
+  }
+
+  // Exercise real Settings and GitHub reads, but never enumerate/sync other
+  // installations. info() receives already-verified App metadata.
+  const repoClient = Object.create(octokit)
+  repoClient.rest = {
+    ...octokit.rest,
+    apps: { ...octokit.rest.apps, getAuthenticated: async () => ({ data: appInfo }) }
+  }
+  const listRoute = { url: '/app/installations', method: 'GET' }
+  const appClient = {
+    rest: { apps: { listInstallations: { endpoint: { merge: () => listRoute } } } },
+    paginate: async route => {
+      if (route !== listRoute) throw new Error('Phase 22 unexpected App request')
+      return [installation]
+    }
+  }
+  const summaries = []
+  const logger = {
+    trace: () => {},
+    debug: () => {},
+    info: message => { summaries.push(message); log(message) },
+    warn: message => log(`Warning: ${message}`),
+    error: message => logFail(`22: ${message}`)
+  }
+  const readOnlyTestOrg = options => {
+    const request = octokit.request.endpoint(options)
+    const pathname = new URL(request.url).pathname.toLowerCase().replace(/^\/api\/v3/, '')
+    const owner = ORG.toLowerCase()
+    const inOrg = pathname === `/orgs/${owner}` || pathname.startsWith(`/orgs/${owner}/`) ||
+      pathname.startsWith(`/repos/${owner}/`) || pathname === '/installation/repositories'
+    if (request.method !== 'GET' || !inOrg) {
+      throw new Error('Phase 22 blocked a non-read-only or out-of-org request')
+    }
+  }
+  octokit.hook.before('request', readOnlyTestOrg)
+  try {
+    const instance = require('./index')({
+      on: () => {},
+      log: logger,
+      auth: async id => {
+        if (id === undefined) return appClient
+        if (id !== installationId) throw new Error('Phase 22 blocked authentication outside the test organization')
+        return repoClient
+      }
+    }, {})
+    const aggregate = await instance.syncInstallation(true)
+    if (!assert(aggregate?.results?.length === 1, '22: exactly one verified installation returned a result')) return
+    const result = aggregate.results[0]
+    assert(aggregate.errors.length === 0, '22: no aggregate full-sync errors')
+    assert(result.errors.length === 0, '22: real Settings reports no errors')
+    assert(result.nop === true, '22: real Settings ran in NOP mode')
+    assert(result.installation_id === installationId, '22: Settings used the verified installation ID')
+    assert(result.repo.owner.toLowerCase() === ORG.toLowerCase(), '22: Settings used the authorized test-org owner')
+    assert(result.github === repoClient, '22: Settings used the test-org authenticated client')
+    assert(result.processedRepoNames.has(ADMIN_REPO), '22: real installation repository enumeration included the admin repo')
+    assert(summaries.filter(message => message === 'Synced 1 of 1 installation(s); 0 failed').length === 1,
+      '22: successful installation summary logged exactly once')
+  } finally {
+    octokit.hook.remove('request', readOnlyTestOrg)
+  }
+}
+
 async function main () {
+  if (process.env.CRON) {
+    throw new Error('Smoke tests require CRON unset to avoid syncing other installations')
+  }
   const { App } = await import('octokit')
   const app = new App({ appId: APP_ID, privateKey: PRIVATE_KEY })
 
@@ -3610,7 +3690,8 @@ async function main () {
       ['Phase 18: Team include/exclude filters', phase18TeamIncludeExclude],
       ['Phase 19: Bypass actor convergence', phase19BypassActorConvergence],
       ['Phase 20: Custom property exclusions', phase20CustomPropertyExclusions],
-      ['Phase 21: Variable pagination', phase21VariablePagination]
+      ['Phase 21: Variable pagination', phase21VariablePagination],
+      ['Phase 22: Test-org installation full-sync NOP', () => phase22InstallationFullSync(app, installationId)]
     ]
 
     // When --phase is given, only run setup (phase 0) + the requested phase(s).

test/unit/full-sync.test.js

@@ -0,0 +1,118 @@
+const { spawnSync } = require('child_process')
+
+// Run both real entrypoints in a child process, replacing only external clients
+// and Settings work. No App credentials or caller environment reach the child.
+const script = `
+  const Module = require('module')
+  const fs = require('fs')
+  const spec = JSON.parse(fs.readFileSync(0, 'utf8'))
+  const load = Module._load
+  const installations = spec.stages.map((stage, i) => ({
+    id: i + 1, target_type: 'Organization', account: { login: 'org-' + (i + 1) }
+  }))
+  let enumerations = 0
+  const appGithub = {
+    paginate: async () => {
+      if (++enumerations > 1 && spec.enumerationFailure) throw new Error('enumeration failed')
+      return installations
+    },
+    rest: { apps: { listInstallations: { endpoint: { merge: x => x } } } }
+  }
+  const robot = {
+    on: () => {},
+    log: { trace: () => {}, debug: () => {}, info: console.log, error: console.error },
+    auth: async id => {
+      if (id === undefined) return appGithub
+      if (enumerations > 1 && spec.stages[id - 1] === 'auth') throw new Error('auth failed')
+      return { id, rest: { apps: { getAuthenticated: async () => ({ data: { slug: 'test' } }) } } }
+    }
+  }
+  class ConfigManager {
+    constructor (context) { this.context = context }
+    async loadGlobalSettingsYaml () {
+      if (spec.stages[this.context.payload.installation.id - 1] === 'config') throw new Error('config failed')
+      return {}
+    }
+  }
+  const Settings = {
+    handleError: async () => { console.error('config error reported') },
+    syncAll: async (nop, context, repo) => {
+      console.log('SYNC ' + repo.owner + ' auth=' + context.octokit.id + ' nop=' + nop)
+      const stage = spec.stages[context.payload.installation.id - 1]
+      if (stage === 'throw') throw new Error('sync failed')
+      if (stage === 'missing') return undefined
+      return { errors: stage === 'errors' ? [{ msg: 'repository failed', owner: repo.owner }] : [] }
+    }
+  }
+  Module._load = function (name, parent, isMain) {
+    if (name === 'probot') return { createProbot: () => robot }
+    if (name === './lib/settings') return Settings
+    if (name === './lib/configManager') return ConfigManager
+    return load.call(this, name, parent, isMain)
+  }
+  require(process.argv[1])
+`
+
+const run = (stages, nop = false, enumerationFailure = false) => spawnSync(process.execPath, [
+  '-e', script, require.resolve('../../full-sync')
+], {
+  env: {
+    FULL_SYNC_NOP: String(nop),
+    DEPLOYMENT_CONFIG_FILE: 'test/fixtures/no-deployment-settings.yml'
+  },
+  input: JSON.stringify({ stages, enumerationFailure }),
+  encoding: 'utf8',
+  timeout: 10000
+})
+
+describe('full-sync entrypoint', () => {
+  it.each([false, true])('completes all successful installations in NOP=%s and exits zero', nop => {
+    const cli = run(['success', 'success'], nop)
+    expect(cli.error).toBeUndefined()
+    expect(cli.status).toBe(0)
+    expect(cli.stdout).toContain(`SYNC org-1 auth=1 nop=${nop}`)
+    expect(cli.stdout).toContain(`SYNC org-2 auth=2 nop=${nop}`)
+    expect(cli.stdout).toContain('Synced 2 of 2 installation(s); 0 failed')
+    expect(cli.stdout).toContain('Full sync completed successfully.')
+    expect(cli.stderr).toBe('')
+  })
+
+  it.each(['auth', 'config', 'throw', 'missing', 'errors'])(
+    'exits nonzero after %s failure but still syncs later installations',
+    stage => {
+      const cli = run([stage, 'success'])
+      expect(cli.error).toBeUndefined()
+      expect(cli.status).toBe(1)
+      expect(cli.stdout).toContain('SYNC org-2 auth=2 nop=false')
+      expect(cli.stdout).toContain('Synced 1 of 2 installation(s); 1 failed')
+      expect(cli.stderr).toContain('Errors occurred during full sync.')
+      expect(cli.stdout).not.toContain('Full sync completed successfully.')
+    }
+  )
+
+  it('exits nonzero for a real NOP configuration fall-through and still syncs the next installation', () => {
+    const cli = run(['config', 'success'], true)
+    expect(cli.status).toBe(1)
+    expect(cli.stdout).toContain('SYNC org-2 auth=2 nop=true')
+    expect(cli.stderr).toContain('config error reported')
+    expect(cli.stderr).toContain('returned no result')
+    expect(cli.stderr).toContain('Errors occurred during full sync.')
+    expect(cli.stdout).not.toContain('Full sync completed successfully.')
+  })
+
+  it('keeps zero installations nonzero with an explicit diagnostic instead of a TypeError', () => {
+    const cli = run([])
+    expect(cli.status).toBe(1)
+    expect(cli.stderr).toContain('No eligible installations found for full sync.')
+    expect(cli.stdout).not.toContain('TypeError')
+    expect(cli.stdout).not.toContain('Full sync completed successfully.')
+  })
+
+  it('exits nonzero when enumeration fails before any sync', () => {
+    const cli = run(['success'], false, true)
+    expect(cli.status).toBe(1)
+    expect(cli.stdout).toContain('Unexpected error during full sync: Error: enumeration failed')
+    expect(cli.stdout).not.toContain('SYNC ')
+    expect(cli.stdout).not.toContain('Full sync completed successfully.')
+  })
+})