Skip to content

Bug/issue 842 #928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
madkoo wants to merge 3 commits into
base: main-enterprise
Choose a base branch
from
+149 −103
Open

Bug/issue 842 #928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1107316
fix: apply org-level settings before loading repository configurations
madkoo Feb 12, 2026
0051d0e
fix: enhance descriptions and add new properties for security feature…
madkoo Feb 12, 2026
bb1b033
fix: update description for deprecated squash-merge commit title prop…
madkoo Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions lib/plugins/branches.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,18 @@ const Overrides = require('./overrides')
const ignorableFields = []
const previewHeaders = { accept: 'application/vnd.github.hellcat-preview+json,application/vnd.github.luke-cage-preview+json,application/vnd.github.zzzax-preview+json' }
const overrides = {
'contexts': {
'action': 'reset',
'type': 'array'
},
contexts: {
action: 'reset',
type: 'array'
}
}

// GitHub API requires these fields to be present in updateBranchProtection calls
// See: https://docs.github.com/rest/branches/branch-protection#update-branch-protection
const requiredBranchProtectionDefaults = {
required_status_checks: null,
enforce_admins: null,
restrictions: null
}

module.exports = class Branches extends ErrorStash {
Expand Down Expand Up @@ -73,7 +81,7 @@ module.exports = class Branches extends ErrorStash {
resArray.push(new NopCommand(this.constructor.name, this.repo, null, results))
}

Object.assign(params, branch.protection, { headers: previewHeaders })
Object.assign(params, requiredBranchProtectionDefaults, branch.protection, { headers: previewHeaders })

Comment on lines +84 to 85
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
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.
if (this.nop) {
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.repos.updateBranchProtection.endpoint(params), 'Add Branch Protection'))
Expand All @@ -83,7 +91,7 @@ module.exports = class Branches extends ErrorStash {
return this.github.repos.updateBranchProtection(params).then(res => this.log.debug(`Branch protection applied successfully ${JSON.stringify(res.url)}`)).catch(e => { this.logError(`Error applying branch protection ${JSON.stringify(e)}`); return [] })
}).catch((e) => {
if (e.status === 404) {
Object.assign(params, Overrides.removeOverrides(overrides, branch.protection, {}), { headers: previewHeaders })
Object.assign(params, requiredBranchProtectionDefaults, Overrides.removeOverrides(overrides, branch.protection, {}), { headers: previewHeaders })
if (this.nop) {
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.repos.updateBranchProtection.endpoint(params), 'Add Branch Protection'))
return Promise.resolve(resArray)
Expand Down
3 changes: 3 additions & 0 deletions lib/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ class Settings {
const settings = new Settings(nop, context, context.repo(), config, ref)

try {
// Apply org-level settings (e.g., rulesets) first, matching syncAll behavior
await settings.updateOrg()

for (const repo of repos) {
settings.repo = repo
await settings.loadConfigs(repo)
Expand Down
Loading