Skip to content

feat: add app installation plugin for managing GitHub App repo access#1012

Open
decyjphr wants to merge 26 commits into
yadhav/fix-recent-issuesfrom
decyjphr-app-installation-plugin
Open

feat: add app installation plugin for managing GitHub App repo access#1012
decyjphr wants to merge 26 commits into
yadhav/fix-recent-issuesfrom
decyjphr-app-installation-plugin

Conversation

@decyjphr

Copy link
Copy Markdown
Collaborator

Summary

Adds a new plugin system to safe-settings where the target is a GitHub App installation rather than a repository. This enables managing which repos each app has access to (repository_selection) through the same config hierarchy (org → suborg → repo).

Closes #1005

Changes

New files

File Purpose
lib/plugins/appInstallations.js Plugin with delta-based and full sync modes
lib/appOctokitClient.js Enterprise Octokit client with auto-batching at 50 repos per API call
lib/repoSelector.js Repo resolution utility (name, team, custom properties)
test/unit/lib/plugins/appInstallations.test.js Unit tests
test/unit/lib/appOctokitClient.test.js Unit tests
test/unit/lib/repoSelector.test.js Unit tests

Modified files

File Change
lib/settings.js Added syncAppInstallations as separate phase, registered in PLUGINS and ADDITIVE_PLUGINS
index.js Added installation/installation_target webhook handlers for drift detection, enrichContextWithEnterprise helper
test/unit/lib/settings.test.js Updated additive_plugins count from 10 to 11

Design

Delta-based processing (efficient at scale)

  • Suborg change: resolve only that 1 suborg + load its previous version
  • Repo change: just 1 repo name + load previous repo.yml
  • Full sync (cron/manual): full recomputation against live API state

Key features

  • Batching: Enterprise API limits add/remove to 50 repos, handled automatically
  • disable_plugins: Can be disabled at any layer
  • additive_plugins: When enabled, only adds repos, never removes
  • Enterprise slug: From webhook payload, no env var needed
  • Drift detection: installation and installation_target events trigger sync

Testing

All 292 unit tests pass (20 suites). 35 new tests added across 3 test files.

Add a new plugin system where the target is a GitHub App installation
rather than a repository. This enables managing which repos each app
has access to (repository_selection) through the same config hierarchy
(org → suborg → repo) that safe-settings uses for repo-level settings.

New files:
- lib/plugins/appInstallations.js: Plugin with delta + full sync modes
- lib/appOctokitClient.js: Enterprise Octokit client with auto-batching
  at 50 repos per API call
- lib/repoSelector.js: Repo resolution utility (name, team, properties)

Integration:
- settings.js: syncAppInstallations as separate phase after updateOrg
- index.js: installation/installation_target webhook handlers for drift
  detection, enrichContextWithEnterprise helper

Supports disable_plugins and additive_plugins. Enterprise slug is
extracted from webhook payload (no env var needed).

Closes #1005

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread index.js Outdated
decyjphr and others added 2 commits June 25, 2026 12:29
robot.auth() without args returns a JWT app client, not an enterprise
installation-authenticated client. Fix enrichContextWithEnterprise to:
1. Use JWT client to list all installations
2. Find the enterprise installation matching payload.enterprise.slug
3. Call robot.auth(enterpriseInstallation.id) to get the properly
   authenticated client for enterprise API calls.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Store the enterprise installation ID after the first successful lookup
so subsequent calls to enrichContextWithEnterprise can skip the
listInstallations API call and directly call robot.auth(cachedId).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread index.js Outdated
decyjphr and others added 11 commits June 25, 2026 22:23
- Call enrichContextWithEnterprise in syncSelectedSettings (index.js)
- Call syncAppInstallations with changedSubOrgs/changedRepos in syncSelectedRepos
- Add _buildAppChangesFromDelta helper to extract affected apps from changed
  suborg/repo configs and compute repository_selection per app
- syncAppInstallations now handles three modes: pre-computed delta, config-based
  delta (from changed files), and full sync

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Load previous version of changed suborg/repo configs using loadYamlFromRef
- Compare old vs new app_installations sections to detect:
  - Apps removed from config → unselect all previously targeted repos
  - Targeting criteria changed → unselect repos no longer in scope
- Selection takes precedence over unselection when both apply
- Pass baseRef through syncAppInstallations → _buildAppChangesFromDelta

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Delta mode now skips apps configured as repository_selection: all at org
  level (org 'all' takes precedence — never add/remove repos via delta)
- Fix repo config path used to load previous version from baseRef: use
  CONFIG_PATH/repos/<repo>.yml instead of bare repos/<repo>.yml (the bare
  path 404'd, so repo-level repository_unselection was never computed)
- schema/settings.json: add app_installations top-level property and include
  it in additive_plugins and disable_plugins enums for editor validation

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

- Remove installation.repositories_added/removed handler: an app only
  receives those events for its own installation, so they cannot detect
  drift on managed apps. Drift is reconciled by the scheduled full sync.
- Process repository_unselection before repository_selection (delta and full
  sync) so a repo removed by one config and added by another ends up present
- Add test asserting removal-before-addition ordering
- Document app_installations in README (prerequisites, hierarchy, examples,
  sync behavior, drift note, disable/additive support) and add it to the
  configurable-items and disable_plugins lists

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
…d/remove)

Rewrite appOctokitClient to the documented 2026-03-10 endpoints:
- org-scoped paths under /enterprises/{ent}/apps/organizations/{org}/...
- repository NAMES instead of IDs
- setRepositorySelection toggle for 'all'/'selected'
- PATCH /add and PATCH /remove, batched at 50

Update appInstallations plugin to pass org, drop ID resolution and
all-repo enumeration, use the toggle for 'all', and handle live
current_selection (all<->selected transitions) in full sync.

Thread current_selection through _computeFullAppDesiredState.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document the design and decisions for the app_installations plugin:
enterprise auth as prerequisite, separate sync phase, fixed repo-selection
criteria, org-all precedence, names-based Enterprise API, delta/full sync,
drift limitations, and future target abstraction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@decyjphr decyjphr marked this pull request as ready for review July 6, 2026 22:47
Copilot AI review requested due to automatic review settings July 6, 2026 22:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Introduces an app_installations capability to safe-settings to manage GitHub App installation repository access (enterprise org installations API) as a separate sync phase, including delta-based updates from changed config files and full reconciliation during full sync runs. It also extends reporting to support “non-repo subjects” (apps) so NOP/check output remains readable and repo counts remain accurate.

Changes:

  • Added new app-installation management plugin (app_installations) plus enterprise Octokit wrapper and repo selection utility (name/team/custom properties/all).
  • Wired a new syncAppInstallations() phase into the settings pipeline and updated NOP reporting to support app-as-subject output.
  • Added schema/docs/tests and smoke-test coverage for the new phase.
Show a summary per file
File Description
index.js Adds enterprise context enrichment, startup verification, and installation_target handler; feeds enterprise client/slug into settings sync.
lib/settings.js Adds syncAppInstallations() phase, delta computation, full desired-state computation, and reporting grouping by “subject”.
lib/plugins/appInstallations.js Implements delta/full reconciliation of app installation repo access (including additive mode behavior).
lib/appOctokitClient.js Wraps enterprise org installations API with batching (50) and convenience methods.
lib/repoSelector.js Resolves repo names via fixed criteria (names/globs, teams, custom properties, all).
lib/nopcommand.js Adds optional subject/subjectType to support app-centric reporting without breaking existing plugins.
schema/settings.json Adds app_installations schema and extends additive/disable plugin enums to include app_installations.
README.md Documents app_installations behavior, prerequisites, and config patterns.
docs/adr/README.md Introduces ADR index.
docs/adr/0001-app-installation-plugin.md Captures the design/decisions for the new app-installation feature.
smoke-test.js Adds Phase 17 to exercise app installation management end-to-end against an enterprise.
test/unit/lib/settings.test.js Updates additive plugin expectations and adds delta-build tests for app changes.
test/unit/lib/plugins/appInstallations.test.js Adds unit tests for delta and full sync behaviors, additive mode, and ordering.
test/unit/lib/appOctokitClient.test.js Adds unit tests for enterprise client behavior and batching.
test/unit/lib/repoSelector.test.js Adds unit tests for repo resolution criteria and precedence.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 15/15 changed files
  • Comments generated: 8
  • Review effort level: Low

Comment thread index.js Outdated
Comment thread index.js
Comment thread index.js
Comment thread lib/settings.js
Comment thread lib/plugins/appInstallations.js
Comment thread lib/plugins/appInstallations.js
Comment thread schema/settings.json Outdated
Comment thread docs/adr/0001-app-installation-plugin.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 16/16 changed files
  • Comments generated: 5
  • Review effort level: Low

Comment thread lib/settings.js
Comment thread lib/settings.js
Comment thread lib/settings.js Outdated
Comment thread docs/adr/0001-app-installation-plugin.md
Comment thread index.js
@decyjphr decyjphr requested a review from Copilot July 7, 2026 20:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 16/16 changed files
  • Comments generated: 9
  • Review effort level: Low

Comment thread lib/settings.js
Comment thread lib/settings.js
Comment thread lib/settings.js
Comment thread lib/settings.js
Comment thread test/unit/lib/settings.test.js Outdated
Comment thread test/unit/lib/settings.test.js
Comment thread index.js
Comment thread index.js
Comment thread index.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants