From 7ac3528750b346f181347bb09f6af927a1c0aa14 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Tue, 4 Aug 2026 14:41:48 -0400 Subject: [PATCH] fix(policy): allow sync merges and longer PR titles (#117) Refs: PLAT-62 --- .github/PULL_REQUEST_TEMPLATE.md | 1 + .github/workflows/callable-pr-policy.yaml | 13 ++++- README.md | 2 +- test/pr-policy.test.mjs | 66 +++++++++++++++++++++-- 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 4a8f12b..4d3359a 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -2,6 +2,7 @@ PR conventions - Title format: type(scope): description (DEV-123) - type ∈ feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert, release + - Maximum 120 characters, including the Jira suffix. - Active Jira projects: DEV (product), PLAT (platform), SEC (security). INFRA is a closed archive. - The Jira key is required at the end of the title in parentheses. - Jira-exempt only: Dependabot PRs and permission-controlled emergency reverts. diff --git a/.github/workflows/callable-pr-policy.yaml b/.github/workflows/callable-pr-policy.yaml index d7689d7..28824d8 100644 --- a/.github/workflows/callable-pr-policy.yaml +++ b/.github/workflows/callable-pr-policy.yaml @@ -83,7 +83,7 @@ jobs: function validateTitle(title, isDependabot, jiraMaybeExempt) { const errs = []; - if (title.length > 72) errs.push('Title is ' + title.length + ' chars — max 72'); + if (title.length > 120) errs.push('Title is ' + title.length + ' chars — max 120'); const m = title.match(/^(feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert|release)(\([^)]+\))?(!)?: (.+?)(\s+\((DEV|PLAT|SEC)-\d+\))?$/); if (!m) { errs.push('Title must match: type(scope): description (KEY-NNN). Allowed types: ' + CONV_TYPES.join(' ')); @@ -185,6 +185,12 @@ jobs: return errs; } + function isSyncMergeCommit(commit) { + if (!Array.isArray(commit.parents) || commit.parents.length < 2) return false; + const subject = (commit.commit && commit.commit.message ? commit.commit.message : '').split('\n')[0]; + return /^Merge (?:branch|remote-tracking branch) '[^']+' into \S.+$/.test(subject); + } + function detectAiFooter(text) { return AI_FOOTER_RE.test(text); } @@ -579,6 +585,7 @@ jobs: validateBranch, validateBody, validateCommitSubject, + isSyncMergeCommit, detectAiFooter, isWorkflowFilename, isActionManifestFilename, @@ -706,7 +713,9 @@ jobs: if (!link.includes('rel="next"') || commits.length === 0) commitMore = false; for (const c of commits) { const subject = c.commit.message.split('\n')[0]; - for (const e of validateCommitSubject(subject)) addViolation('Commit ' + c.sha.slice(0, 8) + ': ' + e); + if (!isSyncMergeCommit(c)) { + for (const e of validateCommitSubject(subject)) addViolation('Commit ' + c.sha.slice(0, 8) + ': ' + e); + } if (detectAiFooter(c.commit.message)) addViolation('Commit ' + c.sha.slice(0, 8) + ': AI attribution footer detected'); } commitPage++; diff --git a/README.md b/README.md index 5fc4ece..a892786 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Organization-level GitHub configuration for Sea Haven Industries. ### PR title -`type(scope): description (DEV-123)` — the Jira key is required at the end in parentheses. Active projects: **DEV** (product), **PLAT** (platform), **SEC** (security). INFRA is a closed archive. Jira-exempt only: Dependabot PRs and permission-controlled emergency reverts. +`type(scope): description (DEV-123)` — maximum 120 characters, including the Jira suffix. The Jira key is required at the end in parentheses. Active projects: **DEV** (product), **PLAT** (platform), **SEC** (security). INFRA is a closed archive. Jira-exempt only: Dependabot PRs and permission-controlled emergency reverts. ### PR body diff --git a/test/pr-policy.test.mjs b/test/pr-policy.test.mjs index cad939e..9d0b964 100644 --- a/test/pr-policy.test.mjs +++ b/test/pr-policy.test.mjs @@ -92,7 +92,7 @@ async function loadValidators() { assert.ok(v && typeof v === 'object', 'PR_POLICY_TEST escape must return an object'); for (const fn of [ 'validateTitle', 'getJiraKey', 'validateBranch', 'validateBody', - 'validateCommitSubject', 'detectAiFooter', 'isWorkflowFilename', + 'validateCommitSubject', 'isSyncMergeCommit', 'detectAiFooter', 'isWorkflowFilename', 'validateWorkflowContent', 'parseRetryAfterMs', 'checkCommitLimit', 'checkFilesLimit', 'normalizeViolationFingerprint', 'filterNewViolations', 'fnv1a32', 'stripHash', 'classifyFileStatus', @@ -132,10 +132,30 @@ describe('validateTitle', () => { assert.ok(errs.some(e => e.includes('type') || e.includes('match')), 'should mention type'); }); - it('rejects title over 72 chars', () => { - const long = 'feat: ' + 'a'.repeat(60) + ' (DEV-1)'; + it('accepts a human PR title at exactly 120 chars', () => { + const long = 'feat: ' + 'a'.repeat(106) + ' (DEV-1)'; + assert.equal(long.length, 120); + assert.deepEqual(v.validateTitle(long, false), []); + }); + + it('accepts a Dependabot PR title at exactly 120 chars', () => { + const long = 'chore(deps): ' + 'a'.repeat(107); + assert.equal(long.length, 120); + assert.deepEqual(v.validateTitle(long, true), []); + }); + + it('rejects a human PR title at exactly 121 chars', () => { + const long = 'feat: ' + 'a'.repeat(107) + ' (DEV-1)'; + assert.equal(long.length, 121); const errs = v.validateTitle(long, false); - assert.ok(errs.some(e => e.includes('72')), 'should mention 72 char limit'); + assert.ok(errs.some(e => e.includes('120')), 'should mention 120 char limit'); + }); + + it('rejects a Dependabot PR title at exactly 121 chars', () => { + const long = 'chore(deps): ' + 'a'.repeat(108); + assert.equal(long.length, 121); + const errs = v.validateTitle(long, true); + assert.ok(errs.some(e => e.includes('120')), 'should mention 120 char limit'); }); it('rejects title ending with a period (Dependabot, no Jira required)', () => { @@ -382,6 +402,44 @@ describe('validateCommitSubject', () => { }); }); +describe('isSyncMergeCommit', () => { + it('recognizes a generated branch synchronization merge', () => { + const commit = { + parents: [{ sha: 'a' }, { sha: 'b' }], + commit: { message: "Merge branch 'main' into chore/pr-policy-rollout" }, + }; + assert.equal(v.isSyncMergeCommit(commit), true); + }); + + it('recognizes a generated remote-tracking synchronization merge', () => { + const commit = { + parents: [{ sha: 'a' }, { sha: 'b' }], + commit: { message: "Merge remote-tracking branch 'origin/main' into fix/example" }, + }; + assert.equal(v.isSyncMergeCommit(commit), true); + }); + + it('does not exempt an arbitrary multi-parent commit', () => { + const commit = { + parents: [{ sha: 'a' }, { sha: 'b' }], + commit: { message: 'Update policy files' }, + }; + assert.equal(v.isSyncMergeCommit(commit), false); + }); + + it('does not exempt a single-parent commit with a merge-shaped subject', () => { + const commit = { + parents: [{ sha: 'a' }], + commit: { message: "Merge branch 'main' into fix/example" }, + }; + assert.equal(v.isSyncMergeCommit(commit), false); + }); + + it('does not exempt commits with missing parent metadata', () => { + assert.equal(v.isSyncMergeCommit({}), false); + }); +}); + // ── detectAiFooter ──────────────────────────────────────────────────────────── describe('detectAiFooter', () => {