Skip to content

Bug/issue 842#928

Open
madkoo wants to merge 3 commits intogithub:main-enterprisefrom
madkoo:bug/issue-842
Open

Bug/issue 842#928
madkoo wants to merge 3 commits intogithub:main-enterprisefrom
madkoo:bug/issue-842

Conversation

@madkoo
Copy link

@madkoo madkoo commented Feb 12, 2026

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:

  • Ensured required fields (required_status_checks, enforce_admins, restrictions) are always included when updating branch protection to comply with GitHub API requirements. [1] [2] [3]
  • Changed settings application order so organization-level settings (e.g., rulesets) are applied before repository-level settings, matching the behavior of full sync operations.

Schema Enhancements for Security and Reviews:

  • Added support for new security-related settings in the schema: code_security and secret_scanning_ai_detection, each with a status property. [1] [2]
  • Introduced the required_reviewers beta feature in branch protection rules, allowing configuration of required reviewing teams and file patterns.
  • Added allowed_merge_methods to specify which merge methods are permitted in branch protection parameters.

Schema Documentation and Deprecation Updates:

  • Updated descriptions for several fields to clarify deprecation or "closing down" status, including use_squash_pr_title_as_default, permission, and contexts. [1] [2] [3]
  • Improved and clarified documentation for schema properties, including maintainers, rule condition descriptions, and rule naming. [1] [2] [3] [4]

Ruleset and Branch Protection Schema Adjustments:

  • Added support for a new repository value in ruleset target enum and clarified conditions for repository policy rulesets. [1] [2]
  • Enhanced bypass actor configuration: clarified actor_id requirements, expanded bypass_mode options to include exempt, and updated related descriptions. [1] [2]

Removal of Deprecated Schema Features:

  • Removed the merge_queue rule from the schema, reflecting its deprecation or lack of support.

Copilot AI review requested due to automatic review settings February 12, 2026 08:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 updateBranchProtection requests 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_queue rule).

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.

"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.",
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
"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.**",

Copilot uses AI. Check for mistakes.
"squash",
"rebase"
]
}
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
}
},
"minItems": 1,
"uniqueItems": true

Copilot uses AI. Check for mistakes.
Comment on lines +84 to 85
Object.assign(params, requiredBranchProtectionDefaults, branch.protection, { headers: previewHeaders })

Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +84 to 85
Object.assign(params, requiredBranchProtectionDefaults, branch.protection, { headers: previewHeaders })

Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
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.

Safe-settings doesn't create branch protection and org rulesets

1 participant