Conversation
There was a problem hiding this comment.
Pull request overview
Updates Safe-Settings’ branch protection/ruleset handling and the settings schema to align with newer GitHub API expectations and newly supported configuration options.
Changes:
- Ensures
updateBranchProtectionrequests always include GitHub-required top-level keys. - Applies org-level settings (rulesets) before repo-level settings in selected-repo syncs.
- Extends the JSON schema with new security/review/ruleset options and documentation updates (including removing deprecated
merge_queuerule).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| schema/dereferenced/settings.json | Adds/updates schema fields for security settings, PR review/ruleset features, and clarifies/deprecates documentation. |
| lib/settings.js | Changes sync order so org-level settings are applied first in syncSelectedRepos. |
| lib/plugins/branches.js | Adds required branch protection fields into update payloads to satisfy GitHub API requirements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
schema/dereferenced/settings.json
Outdated
| "use_squash_pr_title_as_default": { | ||
| "type": "boolean", | ||
| "description": "Either `true` to allow squash-merge commits to use pull request title, or `false` to use commit message. **This property has been deprecated. Please use `squash_merge_commit_title` instead.", | ||
| "description": "Either `true` to allow squash-merge commits to use pull request title, or `false` to use commit message. **This property is closing down. Please use `squash_merge_commit_title` instead.", |
There was a problem hiding this comment.
The description for use_squash_pr_title_as_default starts a bold section with ** but doesn’t close it, which will render incorrectly in Markdown-based schema docs. Add the closing ** (or remove the emphasis) so the description formatting is valid.
| "description": "Either `true` to allow squash-merge commits to use pull request title, or `false` to use commit message. **This property is closing down. Please use `squash_merge_commit_title` instead.", | |
| "description": "Either `true` to allow squash-merge commits to use pull request title, or `false` to use commit message. **This property is closing down. Please use `squash_merge_commit_title` instead.**", |
| "squash", | ||
| "rebase" | ||
| ] | ||
| } |
There was a problem hiding this comment.
allowed_merge_methods says “At least one option must be enabled”, but the schema doesn’t enforce that (no minItems: 1). Add minItems: 1 (and consider uniqueItems: true) so invalid configs are rejected by schema validation instead of failing later.
| } | |
| }, | |
| "minItems": 1, | |
| "uniqueItems": true |
| Object.assign(params, requiredBranchProtectionDefaults, branch.protection, { headers: previewHeaders }) | ||
|
|
There was a problem hiding this comment.
requiredBranchProtectionDefaults sets required_status_checks, enforce_admins, and restrictions to null for all update calls. Because MergeDeep.compareDeep explicitly ignores deletions for object properties not present in the config, this change can unintentionally clear existing branch protection fields that the config omitted (e.g., existing restrictions would be sent as null). Instead, when branch protection already exists, populate these required keys from the current protection response (converted to the shape expected by updateBranchProtection) and only fall back to null defaults in the 404/no-existing-protection path.
| Object.assign(params, requiredBranchProtectionDefaults, branch.protection, { headers: previewHeaders }) | ||
|
|
There was a problem hiding this comment.
This change alters the shape of the updateBranchProtection payload (it now always includes required_status_checks / enforce_admins / restrictions). There are existing unit tests for lib/plugins/branches.js that assert the exact request body; they need to be updated (and ideally extended) to cover the new required-field behavior so CI doesn’t fail and the intended semantics are locked in.
…erty in settings.json
Fixes #842
This pull request introduces several updates to branch protection logic, repository/org-level settings application, and the configuration schema for repository and organization settings. The main changes improve compatibility with GitHub's API requirements, enhance the schema to support new security and review features, and refine documentation for clarity and deprecation notices.
Branch Protection Logic and Settings Application:
required_status_checks,enforce_admins,restrictions) are always included when updating branch protection to comply with GitHub API requirements. [1] [2] [3]Schema Enhancements for Security and Reviews:
code_securityandsecret_scanning_ai_detection, each with astatusproperty. [1] [2]required_reviewersbeta feature in branch protection rules, allowing configuration of required reviewing teams and file patterns.allowed_merge_methodsto specify which merge methods are permitted in branch protection parameters.Schema Documentation and Deprecation Updates:
use_squash_pr_title_as_default,permission, andcontexts. [1] [2] [3]maintainers, rule condition descriptions, and rule naming. [1] [2] [3] [4]Ruleset and Branch Protection Schema Adjustments:
repositoryvalue in rulesettargetenum and clarified conditions for repository policy rulesets. [1] [2]actor_idrequirements, expandedbypass_modeoptions to includeexempt, and updated related descriptions. [1] [2]Removal of Deprecated Schema Features:
merge_queuerule from the schema, reflecting its deprecation or lack of support.