From a1e401df008921b0273721580301de11c802a457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliv=C3=A9r=20Kecskem=C3=A9ty?= Date: Tue, 20 Oct 2026 00:31:29 +0200 Subject: [PATCH] Read merge settings through GraphQL in the drift check --- README.md | 2 +- scripts/apply-settings.mjs | 14 +++++++++--- scripts/settings.mjs | 20 +++++++++++++++-- scripts/settings.test.mjs | 44 ++++++++++++++++++++++++++++++-------- 4 files changed, 65 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index e32785b..7788631 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ node scripts/apply-settings.mjs # fix them in ever node scripts/apply-settings.mjs --dry-run BSStudio/backstage # one repo ``` -Settings must not drift unnoticed: `.github/workflows/settings-drift.yml` runs the dry run every Monday morning and fails when anything differs. It reads the repos with a token from the read-only "BSStudio settings check" GitHub App (Administration: read), which is installed on the repos in `settings/repos.json`; a repo added to that file needs the App installed too. Fixing drift stays a local run of the script. GitHub turns scheduled workflows off after 60 days without activity in the repo and emails about it; Renovate's monthly PRs normally keep it active. +Settings must not drift unnoticed: `.github/workflows/settings-drift.yml` runs the dry run every Monday morning and fails when anything differs. It reads the repos with a token from the read-only "BSStudio settings check" GitHub App (Administration: read), which is installed on the repos in `settings/repos.json`; a repo added to that file needs the App installed too. The merge settings are read through GraphQL, because the REST API shows them only to tokens that can write the repo's contents. Fixing drift stays a local run of the script. GitHub turns scheduled workflows off after 60 days without activity in the repo and emails about it; Renovate's monthly PRs normally keep it active. A named repo must be listed in `settings/repos.json`. A run writes only what differs, then reads the repo again and reports anything GitHub did not take. Run it as an admin of the listed repos: without admin rights GitHub leaves some settings out of its answers (they show as `missing`) and refuses the writes. A failed call is reported with the `gh` command that failed. diff --git a/scripts/apply-settings.mjs b/scripts/apply-settings.mjs index a584cb2..f490699 100644 --- a/scripts/apply-settings.mjs +++ b/scripts/apply-settings.mjs @@ -1,5 +1,5 @@ import { gh, readJson } from "./io.mjs"; -import { currentSettings, desiredSettings, diffSettings, formatEntry, selectRepos, writes } from "./settings.mjs"; +import { currentSettings, desiredSettings, diffSettings, formatEntry, MERGE_QUERY, selectRepos, writes } from "./settings.mjs"; const usage = "usage: node scripts/apply-settings.mjs [--dry-run] [owner/repo]..."; const args = process.argv.slice(2); @@ -20,14 +20,22 @@ const get = async (path) => { const text = await gh(["api", path]); return text.trim() === "" ? null : JSON.parse(text); }; +const getMerge = async (repo) => { + const [owner, name] = repo.split("/"); + const text = await gh(["api", "graphql", "-f", `query=${MERGE_QUERY}`, "-f", `owner=${owner}`, "-f", `name=${name}`]); + const body = JSON.parse(text); + if (!body.data?.repository) throw new Error(`graphql ${repo}: ${body.errors?.map((error) => error.message).join("; ") ?? "no repository in the answer"}`); + return body.data.repository; +}; const readCurrent = async (repo) => { - const [repository, workflow, securityFixes, configuration] = await Promise.all([ + const [repository, merge, workflow, securityFixes, configuration] = await Promise.all([ get(`repos/${repo}`), + getMerge(repo), get(`repos/${repo}/actions/permissions/workflow`), get(`repos/${repo}/automated-security-fixes`), get(`repos/${repo}/code-security-configuration`), ]); - return currentSettings({ repository, workflow, securityFixes, configuration }); + return currentSettings({ repository, merge, workflow, securityFixes, configuration }); }; async function apply(repo) { diff --git a/scripts/settings.mjs b/scripts/settings.mjs index c3cee18..0bb0e74 100644 --- a/scripts/settings.mjs +++ b/scripts/settings.mjs @@ -18,10 +18,26 @@ export function desiredSettings(baseline, override) { } // A "removed" or "detached" status still names the configuration the repo had. -export function currentSettings({ repository, workflow, securityFixes, configuration }) { +// GraphQL names of the merge settings, keyed by their REST names. +const MERGE_FIELDS = { + allow_squash_merge: "squashMergeAllowed", + allow_merge_commit: "mergeCommitAllowed", + allow_rebase_merge: "rebaseMergeAllowed", + allow_auto_merge: "autoMergeAllowed", + delete_branch_on_merge: "deleteBranchOnMerge", + squash_merge_commit_title: "squashMergeCommitTitle", + squash_merge_commit_message: "squashMergeCommitMessage", + allow_update_branch: "allowUpdateBranch", +}; + +export const MERGE_QUERY = `query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { ${Object.values(MERGE_FIELDS).join(" ")} } }`; + +// Merge settings come from GraphQL: the REST repository leaves them out for tokens without contents write, such as the read-only drift check's. +export function currentSettings({ repository, merge, workflow, securityFixes, configuration }) { const applies = configuration !== null && ["attached", "enforced"].includes(configuration.status); + const mergeSettings = merge ? Object.fromEntries(Object.entries(MERGE_FIELDS).map(([rest, graphql]) => [rest, merge[graphql]])) : {}; return { - repository, + repository: { ...repository, ...mergeSettings }, workflow, dependabotSecurityUpdates: securityFixes.enabled, securityConfiguration: applies ? configuration.configuration.name : null, diff --git a/scripts/settings.test.mjs b/scripts/settings.test.mjs index 6253790..03abe59 100644 --- a/scripts/settings.test.mjs +++ b/scripts/settings.test.mjs @@ -7,22 +7,24 @@ const baseline = readJson("settings/baseline.json"); const repos = readJson("settings/repos.json"); const repo = "BSStudio/request-manager"; -// request-manager's responses, trimmed to a few fields more than the baseline names. +// request-manager's responses as a read-only token sees them: REST leaves out the merge settings, GraphQL has them. const responses = { repository: { full_name: "BSStudio/request-manager", - allow_squash_merge: true, - allow_merge_commit: false, - allow_rebase_merge: false, - squash_merge_commit_title: "PR_TITLE", - squash_merge_commit_message: "BLANK", - delete_branch_on_merge: true, - allow_update_branch: true, - allow_auto_merge: true, has_issues: true, has_wiki: false, has_projects: true, }, + merge: { + squashMergeAllowed: true, + mergeCommitAllowed: false, + rebaseMergeAllowed: false, + autoMergeAllowed: true, + deleteBranchOnMerge: true, + squashMergeCommitTitle: "PR_TITLE", + squashMergeCommitMessage: "BLANK", + allowUpdateBranch: true, + }, workflow: { default_workflow_permissions: "write", can_approve_pull_request_reviews: true }, securityFixes: { enabled: false, paused: false }, configuration: { status: "removed", configuration: { id: 23162, name: "Legacy" } }, @@ -88,6 +90,30 @@ test("reads the API responses in the baseline's shape", () => { assert.equal(current.dependabotSecurityUpdates, false); }); +test("merge settings come from GraphQL", () => { + const { repository } = currentSettings(responses); + assert.equal(repository.allow_squash_merge, true); + assert.equal(repository.allow_merge_commit, false); + assert.equal(repository.allow_rebase_merge, false); + assert.equal(repository.allow_auto_merge, true); + assert.equal(repository.delete_branch_on_merge, true); + assert.equal(repository.squash_merge_commit_title, "PR_TITLE"); + assert.equal(repository.squash_merge_commit_message, "BLANK"); + assert.equal(repository.allow_update_branch, true); + assert.equal(repository.has_wiki, false); +}); + +test("GraphQL wins over REST for merge settings", () => { + const rest = { ...responses.repository, allow_auto_merge: false }; + assert.equal(currentSettings({ ...responses, repository: rest }).repository.allow_auto_merge, true); +}); + +test("without a GraphQL answer the merge settings are missing", () => { + const { repository } = currentSettings({ ...responses, merge: null }); + assert.equal(repository.allow_squash_merge, undefined); + assert.equal(repository.has_wiki, false); +}); + test("a removed configuration counts as none", () => { assert.equal(currentSettings(responses).securityConfiguration, null); });