Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions __tests__/integration/rulesets.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'] }
Expand Down
37 changes: 37 additions & 0 deletions __tests__/unit/schema.test.js
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down
18 changes: 18 additions & 0 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,24 @@ export function getChatopsRepoConfig() {
return config?.chatops_repo || { enabled: false };
}

/**
* Returns true when `<owner>/<repo>` 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;
Expand Down
7 changes: 6 additions & 1 deletion src/organization.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion src/repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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})`);

Expand Down
12 changes: 10 additions & 2 deletions src/rulesets.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
Expand Down
Loading