diff --git a/__tests__/integration/rulesets.test.js b/__tests__/integration/rulesets.test.js index af6150b..2a1acb0 100644 --- a/__tests__/integration/rulesets.test.js +++ b/__tests__/integration/rulesets.test.js @@ -57,6 +57,20 @@ describe('translateBranchProtectionToRuleset', () => { expect(pr.parameters.required_review_thread_resolution).toBe(true); }); + it('omits required_status_checks rule entirely when contexts is empty (Rulesets API rejects empty)', () => { + const r = translateBranchProtectionToRuleset({ + required_status_checks: { strict: true, contexts: [] } + }); + expect(r.rules.find((x) => x.type === 'required_status_checks')).toBeUndefined(); + }); + + it('omits required_status_checks rule when contexts is missing', () => { + const r = translateBranchProtectionToRuleset({ + required_status_checks: { strict: true } + }); + expect(r.rules.find((x) => x.type === 'required_status_checks')).toBeUndefined(); + }); + it('emits required_status_checks with strict policy and contexts', () => { const r = translateBranchProtectionToRuleset({ required_status_checks: { strict: true, contexts: ['ci/build', 'ci/test'] } diff --git a/__tests__/unit/schema.test.js b/__tests__/unit/schema.test.js index 693f3b5..006c6be 100644 --- a/__tests__/unit/schema.test.js +++ b/__tests__/unit/schema.test.js @@ -1,4 +1,41 @@ import { validateConfig } from '../../src/schema.js'; +import { _setConfigForTesting, isControlSurfaceRepo } from '../../src/config.js'; + +describe('isControlSurfaceRepo', () => { + beforeEach(() => { _setConfigForTesting({}); }); + afterEach(() => { _setConfigForTesting({}); }); + + it('returns false when no chatops_repo / controller_repo configured', () => { + expect(isControlSurfaceRepo('pulseengine/temper')).toBe(false); + }); + + it('matches chatops_repo.repo when enabled', () => { + _setConfigForTesting({ + chatops_repo: { enabled: true, repo: 'pulseengine/temper-ops' } + }); + expect(isControlSurfaceRepo('pulseengine/temper-ops')).toBe(true); + expect(isControlSurfaceRepo('pulseengine/temper')).toBe(false); + }); + + it('does NOT match when chatops_repo is configured but disabled', () => { + _setConfigForTesting({ + chatops_repo: { enabled: false, repo: 'pulseengine/temper-ops' } + }); + expect(isControlSurfaceRepo('pulseengine/temper-ops')).toBe(false); + }); + + it('matches controller_repo.repo when enabled', () => { + _setConfigForTesting({ + controller_repo: { enabled: true, repo: 'pulseengine/repo-requests' } + }); + expect(isControlSurfaceRepo('pulseengine/repo-requests')).toBe(true); + }); + + it('handles non-string input safely', () => { + expect(isControlSurfaceRepo(undefined)).toBe(false); + expect(isControlSurfaceRepo(null)).toBe(false); + }); +}); describe('validateConfig', () => { it('accepts valid config', () => { diff --git a/src/config.js b/src/config.js index f562382..533978d 100644 --- a/src/config.js +++ b/src/config.js @@ -164,6 +164,24 @@ export function getChatopsRepoConfig() { return config?.chatops_repo || { enabled: false }; } +/** + * Returns true when `/` is a control-surface repo owned by + * Temper itself (the chatops admin repo or the issue-form controller). + * These repos are *not* org code; they're meta. They should be skipped by + * `configureRepository` and `synchronizeAllRepositories` to avoid: + * - branch-protection 403s (private repos on free plans) + * - dependabot/template noise on a repo with no code + * - the bot configuring itself into a corner + */ +export function isControlSurfaceRepo(fullName) { + if (typeof fullName !== 'string') return false; + const chatops = config?.chatops_repo; + const ctrl = config?.controller_repo; + if (chatops?.enabled && chatops?.repo === fullName) return true; + if (ctrl?.enabled && ctrl?.repo === fullName) return true; + return false; +} + export function getRequiredSignaturesFlag(protectionConfig = {}) { if (typeof protectionConfig.require_signed_commits === 'boolean') { return protectionConfig.require_signed_commits; diff --git a/src/organization.js b/src/organization.js index 3ed1354..8ce259c 100644 --- a/src/organization.js +++ b/src/organization.js @@ -1,4 +1,4 @@ -import { getTargetIssueLabels } from './config.js'; +import { getTargetIssueLabels, isControlSurfaceRepo } from './config.js'; import { getLogger } from './logger.js'; import { configureRepository } from './repository.js'; import { checkExistingDependabotConfig } from './dependabot.js'; @@ -37,6 +37,11 @@ async function synchronizeAllRepositories(octokit, org) { continue; } + if (isControlSurfaceRepo(repo.full_name)) { + getLogger().info(`Skipping control-surface repository: ${repo.full_name}`); + continue; + } + getLogger().info(`Processing repository: ${repo.full_name}`); const configResult = await configureRepository(octokit, repo); if (!configResult.success) { diff --git a/src/repository.js b/src/repository.js index 1f40d3e..289ff04 100644 --- a/src/repository.js +++ b/src/repository.js @@ -6,7 +6,8 @@ import { getBranchProtectionConfig, mergePullRequestRules, getTargetIssueLabels, - getRulesetConfig + getRulesetConfig, + isControlSurfaceRepo } from './config.js'; import { applyBranchProtection } from './branch-protection.js'; import { applyRulesetFromBranchProtection } from './rulesets.js'; @@ -79,8 +80,17 @@ async function configureRepository( const repoInfo = normalizeRepoInput(repoOrOwner, maybeRepo); const owner = repoInfo.owner.login; const repo = repoInfo.name; + const fullName = `${owner}/${repo}`; const defaultBranch = getDefaultBranch(repoInfo); + if (isControlSurfaceRepo(fullName)) { + getLogger().info( + { repo: fullName }, + 'Skipping configuration: control-surface repo (chatops_repo / controller_repo)' + ); + return { success: true, skipped: 'control-surface' }; + } + try { getLogger().info(`Configuring repository: ${owner}/${repo} (skipBranchScopedWork=${skipBranchScopedWork})`); diff --git a/src/rulesets.js b/src/rulesets.js index e0fe355..1aac029 100644 --- a/src/rulesets.js +++ b/src/rulesets.js @@ -53,12 +53,20 @@ export function translateBranchProtectionToRuleset(bpConfig = {}, name = TEMPER_ } const checks = bpConfig.required_status_checks; - if (checks && checks !== null) { + // The Rulesets API rejects `required_status_checks` rules with empty + // `required_status_checks` arrays ("Expected at least 1 elements, got 0"). + // Legacy branch-protection accepts `contexts: []`; rulesets do not. + // When the user has no specific named checks, omit the rule entirely — + // emitting a half-valid rule causes the entire ruleset POST to 422 and + // we silently fall back to legacy. Better to ship the ruleset without + // this rule than to ship no ruleset at all. + const hasContexts = checks && Array.isArray(checks.contexts) && checks.contexts.length > 0; + if (checks && checks !== null && hasContexts) { rules.push({ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: checks.strict ?? false, - required_status_checks: (checks.contexts || []).map((c) => + required_status_checks: checks.contexts.map((c) => typeof c === 'string' ? { context: c } : c ) }