Yadhav/fix recent issues#990
Draft
decyjphr wants to merge 71 commits into
Draft
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Introduced a new "disable_plugins" property in the settings schema to allow disabling specific plugins at various configuration layers. - Each entry can be a plugin name or an object specifying the plugin and its target layer (self, children, all). - Updated smoke-test.js to include interactive mode for manual validation during test phases. - Implemented new test cases for the disable_plugins feature, covering normalization, strip map computation, and integration with updateOrg and updateRepos functions. - Added tests to ensure proper handling of valid and invalid disable_plugins configurations.
…nds survive Without action.msg in the dedup key, multiple disable_plugins NopCommands for the same repo (e.g. skipping 'labels' AND 'teams') all share the same type+repo+plugin+endpoint key and only the first one survives, silently dropping the rest from the PR comment and check-run output. Adding action.msg to the key ensures each unique informational message is retained while still deduplicating exact duplicates. Also adds test 27 to cover this case.
- Introduced `additive_plugins` configuration to allow specific Diffable plugins to run in additive mode, preserving existing entries on GitHub. - Updated `normalizeAdditivePlugins` method to validate and return a set of valid plugin names for additive mode. - Modified `childPluginsList` to include section names for better tracking of additive flags. - Enhanced existing tests to cover new functionality, ensuring proper behavior of plugins in additive mode. - Added integration tests to verify that plugins behave correctly when configured with additive_plugins. - Created a new environment file for webhook proxy configuration.
…roles permissions
- Removed unnecessary comments and streamlined the constructor to enforce uppercase variable names. - Simplified the `find` method to directly return the required variable data. - Updated the `changed` method to directly compare values without additional sorting logic. - Refactored `update`, `add`, and `remove` methods to return NopCommand instances when `nop` is true, preventing actual API calls. - Enhanced unit tests to cover new NopCommand behavior and ensure proper functionality of the Variables plugin. - Introduced phase 13 in smoke tests to validate variable creation, updating, and removal in repository settings. - Added support for phase filtering in smoke tests to allow targeted execution of specific phases.
…AML configurations
Generate safe-settings YAML from existing GitHub configuration for a repo, org, or custom-property-based suborg. - lib/settingsGenerator.js: extraction engine reusing each plugin's find() to read current state and produce config/YAML, with cross-repo intersection for suborg generation. - generate-settings.js: standalone CLI that writes generated YAML to the local filesystem (.sample.yml unless --overwrite); loads .env manually. - index.js + app.yml: repository_dispatch (safe-settings-generate) handler that always opens a PR against the admin repo (never commits to the default branch directly). - Suborg files are named suborgs/<name>_<value>.yml. - README: document generator usage and the PR-only guarantee. - Unit tests for the generator (25 tests).
- Added support for custom repository roles in smoke-test.js, including creation, deletion, and retrieval functions. - Implemented new ruleset management functions for organizations and repositories. - Updated smoke tests to validate the behavior of custom repository roles and rulesets under various scenarios. - Enhanced existing tests to ensure proper handling of additive and disabled plugins for custom repository roles and rulesets. - Introduced new test cases to cover scenarios where suborg configurations change and their impact on repository rulesets. - Improved error handling and logging for better traceability during tests.
…_reviewers drift detection
… object array key reordering
When a suborg.yml file changes its targeting rules (suborgrepos, suborgteams, or suborgproperties), repos that no longer match the updated targeting were not having their suborg-applied settings (e.g. rulesets) removed. This happened because getSubOrgConfigs() only resolves the new targeting, and repos not in the new targeting were skipped in updateRepos(). Fix: Load the previous version of changed suborg config files from the base ref (payload.before for push events, pull_request.base.ref for PR/NOP mode), resolve which repos were previously targeted, compare with current targeting, and process removed repos so diffable's sync() detects and removes orphaned rulesets. Changes: - index.js: Pass payload.after/payload.before as ref/baseRef to syncSelectedSettings in push handler - lib/settings.js: Add getReposRemovedFromSubOrgTargeting() method that compares old vs new targeting to find removed repos - lib/settings.js: Add loadYamlFromRef() helper to load config from a specific git ref without cache interference - lib/settings.js: Update syncSelectedRepos to accept baseRef, identify removed repos, and process them before the suborg loop - test/unit/lib/settings.test.js: Add tests for targeting removal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a sub-test to phase 5 that narrows suborg targeting from suborgteams to suborgrepos (excluding demo-repo-service1), then verifies the suborg ruleset is removed from the dropped repo while retained on the still-targeted repo. Restores team-targeted config afterward for subsequent phases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The plugin was updated to use github.rest.repos.* but the test was still mocking github.repos.*, causing TypeError failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+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>
… function and simplify context enrichment
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>
…po selection logic
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…rations and report unknown apps in logs
…sed repo (loadConfigs(repo) loads a single repo override), so missing entries
…emove operations.
…may mark the run successful) even though enterprise permissions are missing/mis-scoped.
teams.js: - resolveExternalGroupId() now matches external_group names case-insensitively, preventing false "not found" errors caused by casing differences vs. the SCIM-synced Azure AD group name. - syncExternalGroup() no longer treats a missing external group as a fatal error. Newly-created teams whose SCIM group hasn't finished provisioning yet now log a warning and emit a WARNING NopCommand instead of an ERROR, so the PR check surfaces it without failing. settings.js: - handleResults() gains full support for the new WARNING NopCommand type: tracked in stats.warnings, rendered in a dedicated "Warnings" PR comment section, and excluded from the check_run failure condition (only ERROR still flips the conclusion to failure). - childPluginsList(): fixed a validation gap where config entries declared only at suborg/repo level (no matching org-level baseline entry by name) were never passed to configvalidators/ overridevalidators. Only entries matching an org-baseline entry were validated; new entries with no org-level counterpart silently bypassed validation (e.g. an invalid team `permission` value would not be rejected). Now validated against an empty baseConfig too. mergeDeep.js: - Fixed a crash in compareDeepIfVisited(): merging an addition into a modification unconditionally called Object.assign, which threw "Cannot assign to read only property '0' of object '[object String]'" when both values were primitive strings (e.g. the `name` identifying attribute added by addIdentifyingAttribute). This happened whenever an array item had both a modified field (e.g. team `permission`) and a field missing from the target object (e.g. `external_group`). The merge now branches by value type — array: push, object: Object.assign, primitive: direct overwrite. Tests: - teams.test.js updated to assert the WARNING (not ERROR) NopCommand and warning log for the missing-external-group case. - mergeDeep.test.js: added a regression test reproducing the permission-change + new-external_group crash scenario.
…installation-plugin feat: add app installation plugin for managing GitHub App repo access
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Fix external group handling, mergeDeep crash, and config validator gap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Starting with the version 2.1.18 that seemed to be most stable, I've been testing and fixing minor bugs and adding a few critical features and enhancements:
This pull request introduces several major improvements and features to
safe-settings, including enhanced plugin control, suborg re-evaluation logic, expanded documentation, and updated permissions for custom roles. The most important changes are grouped and summarized below.Plugin Control Enhancements
disable_plugins: Adds support for disabling safe-settings plugins at any config layer (deployment, org, suborg, repo) using a newdisable_pluginskey. Includes a detailed strip matrix, cascade rules, and limitations. Documentation and sample settings files have been updated with usage examples. [1]], [2]], [3]], [4]], [5]])additive_plugins: Introduces theadditive_pluginskey at the org level, allowing selected Diffable plugins to only add or update entries, never remove them. This enables merging external changes with policy. Documentation and samples are provided. [1]], [2]])Suborg Re-evaluation Logic
safe-settingsnow re-evaluates suborgs and re-applies settings if a new suborg matches. Includes loop prevention and performance optimizations. ([README.mdR181-R201])Permissions and Integration Updates
app.ymlto request the necessary permissions for managing custom organization and repository roles, supporting new features in GitHub Enterprise Cloud. ([app.ymlR116-R123])index.jsto deduplicate repo/suborg changes and streamline sync operations for selected repos and suborgs. [1]], [2]], [3]])Documentation Improvements
external_groupproperty for teams, describing how to link GitHub teams to external IdP groups via API. ([docs/github-settings/4. teams.mdR51-R63])Other
app.ymlfor formatting. ([app.ymlL28])These changes significantly improve the flexibility, safety, and observability of
safe-settings, especially for large organizations with complex policies.