Conversation
…s in settings.json
…erty in settings.json
…borg levels - Introduced `repos.json` schema for repository-level safe-settings overrides. - Updated `settings.json` schema to include additional properties for org-level configurations. - Created `suborgs.json` schema for suborg-level safe-settings configuration. - Enhanced the build script to dereference all schemas and handle errors during the process.
There was a problem hiding this comment.
Pull request overview
This PR improves safe-settings’ config validation and branch protection syncing by introducing JSON schemas (and a dereferencing build step tied to the GitHub REST API spec), updating REST API versioning references, and enhancing branch protection preservation behavior/tests.
Changes:
- Added new JSON Schemas for org/suborg/repo config files and committed dereferenced schema outputs.
- Updated
script/build-schemato resolve GitHub REST API spec$refs reliably before dereferencing. - Enhanced branch protection syncing/tests to preserve existing GitHub settings (e.g.,
enforce_admins,restrictions) when omitted from config.
Reviewed changes
Copilot reviewed 25 out of 28 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
lib/plugins/branches.js |
Preserves existing branch protection fields and normalizes restrictions/status checks before updates. |
test/unit/lib/plugins/branches.test.js |
Adds/adjusts tests for preserving enforce_admins/restrictions and normalizing restriction arrays. |
script/build-schema |
Fetches GitHub REST API spec URL from schema sources and pre-dereferences it for stable dereferencing. |
schema/settings.json |
Refactors schema into $defs and updates GitHub REST API spec references/version. |
schema/suborgs.json |
Introduces suborg-level schema referencing GitHub REST API spec shapes. |
schema/repos.json |
Introduces repo-level override schema referencing GitHub REST API spec shapes. |
schema/dereferenced/settings.json |
Updated generated dereferenced org-level schema output. |
schema/dereferenced/repos.json |
Added generated dereferenced repo-level schema output. |
lib/settings.js |
Ensures org-level rulesets are applied first when syncing selected repos. |
lib/plugins/custom_properties.js |
Avoids crashes by filtering null property names during normalization. |
lib/plugins/rulesets.js |
Updates API version header and adjusts override metadata. |
test/unit/lib/plugins/rulesets.test.js |
Updates expected API version header. |
lib/plugins/variables.js / lib/plugins/autolinks.js / lib/plugins/collaborators.js |
Updates REST API doc links to the newer API version. |
index.js |
Updates REST API version headers for rename-related content operations. |
docs/** |
Updates documentation links to the newer API version; sample deployment validator script made safer via optional chaining. |
app.yml |
Updates documentation link for Actions variables permission. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // See: https://docs.github.com/rest/branches/branch-protection#update-branch-protection | ||
| const requiredBranchProtectionDefaults = { | ||
| required_status_checks: null, | ||
| enforce_admins: null, |
There was a problem hiding this comment.
updateBranchProtection requires required_pull_request_reviews to be present in the payload (it can be null), but requiredBranchProtectionDefaults only sets required_status_checks, enforce_admins, and restrictions. When both the config and the existing GitHub protection omit required_pull_request_reviews, the request will be sent without it and can fail validation. Add required_pull_request_reviews: null to requiredBranchProtectionDefaults (and ensure it is included in the 404 branch as well).
| enforce_admins: null, | |
| enforce_admins: null, | |
| required_pull_request_reviews: null, |
| if (Array.isArray(protection.required_status_checks.contexts) && protection.required_status_checks.contexts.length === 0) { | ||
| delete protection.required_status_checks.contexts | ||
| } |
There was a problem hiding this comment.
reformatAndReturnBranchProtection deletes required_status_checks.contexts when it is an empty array. The Branch Protection API schema typically requires contexts to be present whenever required_status_checks is non-null, and removing it can cause 422 errors when the repo uses status checks but has no contexts configured. Consider keeping contexts: [] (or only deleting it when checks is present and the API explicitly allows omitting contexts).
| if (Array.isArray(protection.required_status_checks.contexts) && protection.required_status_checks.contexts.length === 0) { | |
| delete protection.required_status_checks.contexts | |
| } |
| await context.octokit.request('GET /repos/{owner}/{repo}/contents/{path}', { | ||
| owner: payload.repository.owner.login, | ||
| repo: env.ADMIN_REPO, | ||
| repo: env.ADMIN_REPO, |
There was a problem hiding this comment.
Trailing whitespace after env.ADMIN_REPO, will fail standard/eslint formatting checks. Remove the extra space.
| repo: env.ADMIN_REPO, | |
| repo: env.ADMIN_REPO, |
| }, | ||
| "force_create": { | ||
| "description": "Force create the repository even if it already exists", | ||
| "type": "boolean" | ||
| }, | ||
| "template": { | ||
| "description": "Name of a template repository to use when creating the repository", | ||
| "type": "string" |
There was a problem hiding this comment.
The force_create description says "Force create the repository even if it already exists", which is misleading: you can’t create a repo that already exists. Align this description with the behavior used elsewhere in the schemas (e.g., "create the repository if it does not exist").
| // Force all names to lowercase to avoid comparison issues. | ||
| normalizeEntries () { | ||
| this.entries = this.entries.map(({ name, value }) => ({ | ||
| name: name.toLowerCase(), | ||
| value | ||
| })) | ||
| this.entries = this.entries | ||
| .filter(({ name }) => name != null) | ||
| .map(({ name, value }) => ({ | ||
| name: name.toLowerCase(), | ||
| value | ||
| })) |
There was a problem hiding this comment.
normalizeEntries() now silently drops custom property entries with name == null. This avoids a crash but can hide invalid config and make debugging harder. Consider logging/stashing a validation error when filtering out invalid entries so users know their input was ignored.
| // Force all names to lowercase to avoid comparison issues. | ||
| normalize (properties) { | ||
| return properties.map(({ property_name: propertyName, value }) => ({ | ||
| name: propertyName.toLowerCase(), | ||
| value | ||
| })) | ||
| return properties | ||
| .filter(({ property_name: propertyName }) => propertyName != null) | ||
| .map(({ property_name: propertyName, value }) => ({ | ||
| name: propertyName.toLowerCase(), | ||
| value | ||
| })) |
There was a problem hiding this comment.
normalize() now silently drops properties with property_name == null. This prevents a crash but can hide malformed GitHub responses/config data. Consider emitting a debug/error log (or error stash entry) when such entries are encountered so the behavior is observable.
| "properties": { | ||
| "repositories": { | ||
| "$ref": "#/$defs/RepositorySettings" | ||
| }, | ||
| "labels": { | ||
| "$ref": "#/$defs/LabelSettings" | ||
| }, |
There was a problem hiding this comment.
Schema uses top-level key repositories, but the actual safe-settings YAML (and Settings.updateRepos) uses repository (singular) for repo settings (see docs/sample-settings/settings.yml and lib/settings.js usage of this.config.repository). As-is, this schema won’t validate real configs. Update the schema to define repository (and optionally alias/accept repositories if needed for backwards compatibility).
| "properties": { | ||
| "repositories": { | ||
| "$ref": "#/$defs/RepositorySettings" | ||
| }, | ||
| "labels": { | ||
| "$ref": "#/$defs/LabelSettings" | ||
| }, |
There was a problem hiding this comment.
Schema uses top-level key repositories, but safe-settings configs use repository (singular) for repo settings (see docs/sample-settings/repo.yml and lib/settings.js uses this.config.repository). This will prevent schema validation from working for repo-level override files. Define repository here (and optionally allow repositories as an alias if required).
| } | ||
| }, | ||
| "repositories": { | ||
| "$ref": "#/$defs/RepositorySettings" | ||
| }, | ||
| "labels": { | ||
| "$ref": "#/$defs/LabelSettings" | ||
| }, |
There was a problem hiding this comment.
Schema uses top-level key repositories, but safe-settings configs use repository (singular) for repo settings (see docs/sample-settings/settings.yml and lib/settings.js uses this.config.repository). This mismatch will cause valid suborg configs to fail validation. Define repository here (and optionally allow repositories as an alias if required).
| >[!TIP] | ||
| >GitHub's API documentation defines these inputs and types: | ||
| >1. [Update a repository variable](https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#update-a-repository-variable) | ||
| >1. [Update a repository variable](https://docs.github.com/en/rest/actions/variables?apiVersion=2026-03-10#update-a-repository-variable) |
There was a problem hiding this comment.
The Branch Protection docs TIP links to the Actions Variables endpoint ("Update a repository variable") instead of the Branch Protection endpoint. Update the link text + URL to point at the branch protection REST API docs for updating branch protection.
| >1. [Update a repository variable](https://docs.github.com/en/rest/actions/variables?apiVersion=2026-03-10#update-a-repository-variable) | |
| >1. [Update branch protection](https://docs.github.com/en/rest/branches/branch-protection?apiVersion=2026-03-10#update-branch-protection) |
Fixes #465
Update schemas to target latest GitHub API
feat(schema): add bypass mode for reviewers in suborgs schema
Introduced a new "mode" property in the suborgs schema to specify the bypass mode for reviewers, with options "ALWAYS" and "EXEMPT".
fix(build-schema): enhance schema dereferencing with GitHub API spec
test(branches): preserve existing branch protection settings
enforce_adminsandrestrictions, are preserved when updating branch protection configurations.