From 75c2a9e260420260f7d8a5aa5ff8e183b0bc7cc0 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Thu, 17 Sep 2026 05:56:49 +0200 Subject: [PATCH] dependabot: never auto-merge a breaking update; never overwrite a repo's own dependabot.yml Two maintainer decisions (temper#71). 1. Auto-merge is armed only for non-breaking updates. Breaking uses the leftmost-non-zero rule Cargo and npm carets use (major; 0.x minor; 0.0.x patch). Every update in a grouped PR is read from Dependabot's own summary lines, never from quoted release notes. Anything unreadable as semver is refused, not assumed safe, and temper comments why it did not arm. Motivation: a green PR is no evidence for a workflow action whose changed code runs only on tag or dispatch (varve: cosign-installer, setup-oras). 2. applyDependabotConfig writes .github/dependabot.yml only where none exists. An existing file is the repository's own -- grouping, held majors, paths CI never runs -- and temper cannot know those details. Decided in one place so /configure-repo, /generate-dependabot and the scheduler cannot differ; a 403 or 500 on the existence check throws rather than counting as absence. Negative controls: disabling the existence guard fails 3 tests; treating any error as absence fails 1; removing the refusal fails the handler test. Full suite 867/867, eslint clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019TNtfRjLNhEz82G2ggeeNu --- __tests__/integration/app.test.js | 112 +++++++++++ __tests__/integration/dependabot.test.js | 55 ++++++ __tests__/unit/dependabot-update-type.test.js | 184 ++++++++++++++++++ src/app.js | 44 ++++- src/dependabot-update-type.js | 134 +++++++++++++ src/dependabot.js | 39 ++++ 6 files changed, 563 insertions(+), 5 deletions(-) create mode 100644 __tests__/unit/dependabot-update-type.test.js create mode 100644 src/dependabot-update-type.js diff --git a/__tests__/integration/app.test.js b/__tests__/integration/app.test.js index da1cb5a..4dc1e8f 100644 --- a/__tests__/integration/app.test.js +++ b/__tests__/integration/app.test.js @@ -252,6 +252,95 @@ describe('app', () => { _setConfigForTesting({}); }); + // ========================================================================= + // pull_request.opened — Dependabot auto-merge is gated on semver (temper#71) + // ========================================================================= + describe('pull_request.opened Dependabot auto-merge', () => { + function prOpenedContext({ sender, title, body = '' }) { + const octokit = createMockOctokit(); + // The handler arms auto-merge through a GraphQL mutation; record it. + octokit.graphql = jest.fn().mockResolvedValue({}); + return { + id: 'delivery-pr-1', + log: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, + octokit, + payload: { + pull_request: { number: 157, node_id: 'PR_node', title, body }, + repository: { + name: 'varve', + default_branch: 'main', + owner: { login: 'pulseengine' } + }, + sender: { login: sender } + } + }; + } + + beforeEach(() => { + _setConfigForTesting({ + auto_merge: { enabled: true, on_dependabot: true, on_bot_users: ['thrum'], merge_method: 'squash' } + }); + }); + + it('arms auto-merge for a non-breaking Dependabot update', async () => { + const { handlers } = setupApp(); + const ctx = prOpenedContext({ + sender: 'dependabot[bot]', + title: 'build(deps): bump flate2 from 1.1.9 to 1.1.10' + }); + await handlers['pull_request.opened'](ctx); + expect(ctx.octokit.graphql).toHaveBeenCalledTimes(1); + expect(ctx.octokit.graphql.mock.calls[0][0]).toContain('enablePullRequestAutoMerge'); + }); + + it('does NOT arm auto-merge for a breaking Dependabot update, and says why', async () => { + const { handlers } = setupApp(); + const ctx = prOpenedContext({ + sender: 'dependabot[bot]', + title: 'build(deps): bump sigstore/cosign-installer from 3.9.1 to 4.1.2' + }); + await handlers['pull_request.opened'](ctx); + + expect(ctx.octokit.graphql).not.toHaveBeenCalled(); + + // Asserted on the ROUTE, not on octokit.issues.createComment: under + // Probot v14 that namespace is undefined for pull_request.opened, so a + // call through it throws, is swallowed, and the explanation silently + // never posts. The route is what production actually uses. + const commentCalls = ctx.octokit.request.mock.calls.filter( + ([route]) => route === 'POST /repos/{owner}/{repo}/issues/{issue_number}/comments' + ); + expect(commentCalls).toHaveLength(1); + expect(commentCalls[0][1].issue_number).toBe(157); + expect(commentCalls[0][1].body).toMatch(/did not enable auto-merge/); + expect(commentCalls[0][1].body).toContain('sigstore/cosign-installer'); + }); + + it('does NOT arm a grouped PR that carries one breaking update', async () => { + const { handlers } = setupApp(); + const ctx = prOpenedContext({ + sender: 'dependabot[bot]', + title: 'chore(deps): Bump the wasmtime group across 1 directory with 2 updates', + body: 'Updates `wasmtime` from 47.0.3 to 48.0.1\nUpdates `wasmtime-wasi` from 47.0.3 to 48.0.1' + }); + await handlers['pull_request.opened'](ctx); + expect(ctx.octokit.graphql).not.toHaveBeenCalled(); + }); + + it('leaves configured bot users on their existing policy', async () => { + // The decision was about Dependabot majors. A configured bot is not + // Dependabot, and a title that happens to look like a major must not + // change how it is treated. + const { handlers } = setupApp(); + const ctx = prOpenedContext({ + sender: 'thrum', + title: 'bump something from 1.0.0 to 2.0.0' + }); + await handlers['pull_request.opened'](ctx); + expect(ctx.octokit.graphql).toHaveBeenCalledTimes(1); + }); + }); + // ========================================================================= // chatops_repo enforcement — slash commands only honoured in admin repo // ========================================================================= @@ -1355,6 +1444,29 @@ describe('app', () => { expect(context.octokit.issues.createComment).toHaveBeenCalledTimes(2); }); + it('says so, rather than claiming success, when the repository already has its own config', async () => { + generateDependabotConfig.mockResolvedValue({ + config: { version: 2, updates: [{ 'package-ecosystem': 'npm', directory: '/' }] }, + ecosystems: [{ ecosystem: 'npm', directory: '/' }], + report: 'Detected 1 ecosystem(s): npm (/)' + }); + applyDependabotConfig.mockResolvedValue({ + applied: false, + reason: 'myorg/myrepo already has .github/dependabot.yml; it is the repository\'s own and temper does not overwrite it' + }); + + const { handlers } = setupApp(); + const context = createIssueCommentContext('/generate-dependabot'); + context.octokit.request.mockImplementationOnce(() => + Promise.resolve({ data: { default_branch: 'main' } }) + ); + await handlers['issue_comment.created'](context); + + const bodies = context.octokit.issues.createComment.mock.calls.map((c) => c[0].body); + expect(bodies.some((b) => b.includes('applied!'))).toBe(false); + expect(bodies[bodies.length - 1]).toContain('does not overwrite'); + }); + it('posts message when no ecosystems detected', async () => { generateDependabotConfig.mockResolvedValue({ config: null, diff --git a/__tests__/integration/dependabot.test.js b/__tests__/integration/dependabot.test.js index bbec3ca..ddfe08e 100644 --- a/__tests__/integration/dependabot.test.js +++ b/__tests__/integration/dependabot.test.js @@ -68,6 +68,61 @@ describe('dependabot', () => { describe('applyDependabotConfig', () => { const depConfig = { version: 2, updates: [{ labels: ['dependencies'] }] }; + // The repository has no dependabot.yml yet: the only case temper writes one. + beforeEach(() => { + octokit.request.mockRejectedValue(Object.assign(new Error('Not Found'), { status: 404 })); + }); + + it('does NOT overwrite a dependabot.yml the repository already has', async () => { + // A repository's own file encodes what temper cannot know: which updates + // to group, which paths PR CI never exercises, which majors to hold. + // pulseengine/varve groups minor+patch; a generic config would undo it. + _setConfigForTesting({}); + octokit.request.mockResolvedValue({ status: 200, data: { content: encodeConfig(depConfig) } }); + + const result = await applyDependabotConfig(octokit, 'owner', 'repo', depConfig); + + expect(upsertRepoFile).not.toHaveBeenCalled(); + expect(createConfigurationPR).not.toHaveBeenCalled(); + expect(result.applied).toBe(false); + expect(result.reason).toMatch(/already has/); + expect(octokit.request).toHaveBeenCalledWith( + 'GET /repos/{owner}/{repo}/contents/{path}', + expect.objectContaining({ owner: 'owner', repo: 'repo', path: '.github/dependabot.yml' }) + ); + }); + + it('does NOT overwrite an existing file via a PR either', async () => { + _setConfigForTesting({ change_strategy: { use_pull_requests: true } }); + octokit.request.mockResolvedValue({ status: 200, data: {} }); + + const result = await applyDependabotConfig(octokit, 'owner', 'repo', depConfig); + + expect(createConfigurationPR).not.toHaveBeenCalled(); + expect(result.applied).toBe(false); + }); + + it('reports that it applied the config when the file was absent', async () => { + _setConfigForTesting({}); + + const result = await applyDependabotConfig(octokit, 'owner', 'repo', depConfig); + + expect(upsertRepoFile).toHaveBeenCalled(); + expect(result.applied).toBe(true); + }); + + it('does not assume the file is absent when the existence check fails for another reason', async () => { + // A 403 or 500 says nothing about whether the file exists. Treating it as + // absence would overwrite a repository's config on a transient error. + _setConfigForTesting({}); + octokit.request.mockRejectedValue(Object.assign(new Error('Server Error'), { status: 500 })); + + await expect( + applyDependabotConfig(octokit, 'owner', 'repo', depConfig) + ).rejects.toThrow('Server Error'); + expect(upsertRepoFile).not.toHaveBeenCalled(); + }); + it('applies config via direct commit when use_pull_requests is false', async () => { _setConfigForTesting({ change_strategy: { use_pull_requests: false } diff --git a/__tests__/unit/dependabot-update-type.test.js b/__tests__/unit/dependabot-update-type.test.js new file mode 100644 index 0000000..d2ef8a2 --- /dev/null +++ b/__tests__/unit/dependabot-update-type.test.js @@ -0,0 +1,184 @@ +import { + parseVersion, + isBreakingUpdate, + extractUpdates, + dependabotAutoMergeDecision +} from '../../src/dependabot-update-type.js'; + +// Every title and body below is copied from a real Dependabot PR in the +// pulseengine org. Synthetic strings would test the parser against what we +// expected Dependabot to write; these test it against what it did write. + +describe('dependabot-update-type', () => { + describe('parseVersion', () => { + it('reads a plain semver', () => { + expect(parseVersion('4.4.0')).toEqual([4, 4, 0]); + }); + + it('drops build metadata (toml writes 0.9.12+spec-1.1.0)', () => { + expect(parseVersion('0.9.12+spec-1.1.0')).toEqual([0, 9, 12]); + }); + + it('pads a one-component version (actions/deploy-pages from 4 to 5)', () => { + expect(parseVersion('4')).toEqual([4, 0, 0]); + }); + + it('strips requirement operators (sphinxcontrib-rust from >=1.2.0)', () => { + expect(parseVersion('>=1.2.0')).toEqual([1, 2, 0]); + }); + + it('strips a trailing full stop from a Bumps summary line', () => { + expect(parseVersion('0.8.0.')).toEqual([0, 8, 0]); + }); + + it('returns null for something that is not a version (a commit SHA)', () => { + expect(parseVersion('3d3c42e5aac5ba805825da76410c181273ba90b1')).toBeNull(); + }); + }); + + describe('isBreakingUpdate', () => { + it('treats a major change as breaking', () => { + expect(isBreakingUpdate([3, 9, 1], [4, 1, 2])).toBe(true); + }); + + it('treats a 0.x minor change as breaking, as Cargo and npm caret ranges do', () => { + // criterion 0.7 -> 0.8, wsc-attestation 0.10 -> 0.11 + expect(isBreakingUpdate([0, 7, 0], [0, 8, 2])).toBe(true); + expect(isBreakingUpdate([0, 10, 0], [0, 11, 0])).toBe(true); + }); + + it('treats a 0.0.x patch change as breaking', () => { + expect(isBreakingUpdate([0, 0, 3], [0, 0, 4])).toBe(true); + }); + + it('does not treat a minor or patch on a 1.x+ line as breaking', () => { + expect(isBreakingUpdate([3, 3, 0], [3, 4, 2])).toBe(false); // ureq + expect(isBreakingUpdate([1, 1, 9], [1, 1, 10])).toBe(false); // flate2 + }); + + it('does not treat a 0.x patch change as breaking', () => { + expect(isBreakingUpdate([0, 7, 3], [0, 7, 4])).toBe(false); + }); + }); + + describe('extractUpdates', () => { + it('reads a single-update title', () => { + expect(extractUpdates('build(deps): bump actions/checkout from 4.4.0 to 7.0.1', '')).toEqual([ + { name: 'actions/checkout', from: '4.4.0', to: '7.0.1' } + ]); + }); + + it('ignores a trailing "in the group" suffix', () => { + const u = extractUpdates( + 'chore(deps): Bump wasmtime-wasi from 48.0.1 to 48.0.2 in the wasmtime group', + '' + ); + expect(u).toEqual([{ name: 'wasmtime-wasi', from: '48.0.1', to: '48.0.2' }]); + }); + + it('reads a "requirement" update with a directory suffix', () => { + const u = extractUpdates( + 'build(deps): update wit-bindgen requirement from 0.60.0 to 0.62.0 in /fuzz', + '' + ); + expect(u).toEqual([{ name: 'wit-bindgen', from: '0.60.0', to: '0.62.0' }]); + }); + + it('reads EVERY update from a grouped PR body, whose title names no versions', () => { + const body = [ + 'Bumps the wasmtime group with 2 updates in the / directory: [wasmtime](https://github.com/bytecodealliance/wasmtime) and [wasmtime-wasi](https://github.com/bytecodealliance/wasmtime).', + 'Updates `wasmtime` from 47.0.3 to 48.0.1', + 'Updates `wasmtime-wasi` from 47.0.3 to 48.0.1' + ].join('\n'); + const u = extractUpdates('chore(deps): Bump the wasmtime group across 1 directory with 2 updates', body); + // Two DIFFERENT packages moving from the same version to the same + // version. Keying on versions alone collapsed these into one. + expect(u).toEqual([ + { name: 'wasmtime', from: '47.0.3', to: '48.0.1' }, + { name: 'wasmtime-wasi', from: '47.0.3', to: '48.0.1' } + ]); + }); + + it('does NOT read "from X to Y" out of embedded release notes', () => { + // Dependabot bodies quote upstream changelogs, which say things like + // "migrate from v1 to v2". Reading those would invent updates. + const body = [ + 'Bumps [ureq](https://github.com/algesten/ureq) from 3.3.0 to 3.4.2.', + '
Changelog', + '
  • Upgrade path from 2.0.0 to 9.0.0 documented
    • ', + '
    ' + ].join('\n'); + const u = extractUpdates('build(deps): bump ureq from 3.3.0 to 3.4.2', body); + expect(u).toEqual([{ name: 'ureq', from: '3.3.0', to: '3.4.2' }]); + }); + }); + + describe('dependabotAutoMergeDecision', () => { + it('allows a patch bump', () => { + const d = dependabotAutoMergeDecision('build(deps): bump flate2 from 1.1.9 to 1.1.10', ''); + expect(d.allow).toBe(true); + }); + + it('allows a minor bump on a stable line', () => { + const d = dependabotAutoMergeDecision('build(deps): bump ureq from 3.3.0 to 3.4.2', ''); + expect(d.allow).toBe(true); + }); + + it('REFUSES a major bump to a signing toolchain', () => { + const d = dependabotAutoMergeDecision( + 'build(deps): bump sigstore/cosign-installer from 3.9.1 to 4.1.2', + '' + ); + expect(d.allow).toBe(false); + expect(d.reason).toMatch(/breaking/i); + expect(d.reason).toContain('sigstore/cosign-installer'); + expect(d.reason).toContain('3.9.1'); + expect(d.reason).toContain('4.1.2'); + }); + + it('REFUSES a 0.x minor bump, which is breaking under Cargo semver', () => { + const d = dependabotAutoMergeDecision('build(deps): bump criterion from 0.7.0 to 0.8.2', ''); + expect(d.allow).toBe(false); + }); + + it('REFUSES a grouped PR when any one update inside it is breaking', () => { + // witness#211: the group is called "wasmtime" and says nothing about + // semver, but it carries 47 -> 48. + const body = [ + 'Updates `wasmtime` from 47.0.3 to 48.0.1', + 'Updates `wasmtime-wasi` from 47.0.3 to 48.0.1' + ].join('\n'); + const d = dependabotAutoMergeDecision( + 'chore(deps): Bump the wasmtime group across 1 directory with 2 updates', + body + ); + expect(d.allow).toBe(false); + }); + + it('allows a grouped PR whose every update is non-breaking', () => { + const body = [ + 'Updates `ureq` from 3.3.0 to 3.4.2', + 'Updates `flate2` from 1.1.9 to 1.1.10' + ].join('\n'); + const d = dependabotAutoMergeDecision( + 'build(deps): bump the cargo-minor-and-patch group with 2 updates', + body + ); + expect(d.allow).toBe(true); + }); + + it('REFUSES when no update can be read at all, rather than assuming it is safe', () => { + const d = dependabotAutoMergeDecision('build(deps): something unrecognised', ''); + expect(d.allow).toBe(false); + expect(d.reason).toMatch(/could not/i); + }); + + it('REFUSES when a version is not semver, e.g. a SHA-only action update', () => { + const d = dependabotAutoMergeDecision( + 'build(deps): bump actions/checkout from 3d3c42e5aac5ba805825da76410c181273ba90b1 to 11d5960a326750d5838078e36cf38b85af677262', + '' + ); + expect(d.allow).toBe(false); + }); + }); +}); diff --git a/src/app.js b/src/app.js index db1446d..19d664c 100644 --- a/src/app.js +++ b/src/app.js @@ -4,6 +4,7 @@ import { fileURLToPath } from 'url'; import { createDashboardHandler, DEPLOY_SHA } from './dashboard.js'; import { getConfig, getControllerRepoConfig, getChatopsRepoConfig } from './config.js'; import { getLogger, setLogger } from './logger.js'; +import { dependabotAutoMergeDecision } from './dependabot-update-type.js'; import { configureRepository } from './repository.js'; import { checkOrganizationMembership, @@ -642,11 +643,13 @@ function registerApp(app, options = {}) { body }); - await applyDependabotConfig(context.octokit, owner, repo, result.config); + const applied = await applyDependabotConfig(context.octokit, owner, repo, result.config); await issueOps.createComment(context.octokit,{ owner, repo, issue_number: issueNumber, - body: '✅ Dependabot configuration applied!' + body: applied?.applied === false + ? `Not applied: ${applied.reason}. The configuration above is for reference only.` + : '✅ Dependabot configuration applied!' }); } } catch (error) { @@ -854,7 +857,36 @@ function registerApp(app, options = {}) { bot => sender === bot || sender === bot + "[bot]" ); - if (isDependabot || isBotUser) { + // Dependabot PRs auto-merge only when every update is non-breaking + // (temper#71). A major — or a 0.x minor, which Cargo and npm treat as + // breaking — waits for a human. For a workflow action a green PR can be + // no evidence at all: an action used only by a tag-triggered release or a + // manually dispatched deposit goes green without ever executing. + // Configured bot users are not Dependabot and keep their existing policy. + let refusedDependabot = false; + if (isDependabot) { + const decision = dependabotAutoMergeDecision(pr.title, pr.body); + if (!decision.allow) { + refusedDependabot = true; + getLogger().info({ pr: pr.number, reason: decision.reason }, "Not enabling auto-merge"); + try { + // issueOps, not octokit.rest.issues: that namespace is undefined + // for pull_request.opened under Probot v14, so a direct call throws, + // lands in the catch below, and the explanation is silently never + // posted while the refusal itself still works. + await issueOps.createComment(context.octokit, { + owner, + repo, + issue_number: pr.number, + body: `temper did not enable auto-merge: ${decision.reason}.` + }); + } catch (err) { + getLogger().warn({ pr: pr.number, err: err.message }, "Could not explain skipped auto-merge"); + } + } + } + + if ((isDependabot && !refusedDependabot) || isBotUser) { const mergeMethod = autoMerge.merge_method || "squash"; getLogger().info({ pr: pr.number, sender, mergeMethod }, "Enabling auto-merge"); @@ -1097,8 +1129,10 @@ function initScheduler(app) { if (result.config) { const labels = extractLabelsFromConfig(result.config); if (labels.length > 0) await ensureLabelsExist(kit, owner, repo, labels); - await applyDependabotConfig(kit, owner, repo, result.config); - logger.info(`Scheduler: applied dependabot config to ${owner}/${repo}`); + const applied = await applyDependabotConfig(kit, owner, repo, result.config); + logger.info(applied?.applied === false + ? `Scheduler: ${applied.reason}` + : `Scheduler: applied dependabot config to ${owner}/${repo}`); } else { logger.info(`Scheduler: no ecosystems detected in ${owner}/${repo}, skipping`); } diff --git a/src/dependabot-update-type.js b/src/dependabot-update-type.js new file mode 100644 index 0000000..6d6f0cc --- /dev/null +++ b/src/dependabot-update-type.js @@ -0,0 +1,134 @@ +/** + * Decide whether a Dependabot PR may be auto-merged, from what it updates. + * + * Policy (maintainer decision, temper#71): auto-merge non-breaking updates + * only. A breaking update waits for a human. + * + * Why it cannot be left to CI. For a library, a green PR is decent evidence + * a bump works. For a workflow action it can be none at all: on + * pulseengine/varve, `setup-oras` runs only in a `workflow_dispatch` deposit + * workflow and `cosign sign-blob` only in a tag-triggered release, so a major + * bump to either goes green without its changed code ever executing — and + * the first failure lands after a release is tagged. + * + * Why the decision is conservative. Anything that cannot be read as semver — + * an unrecognised title, a SHA-only action update — is REFUSED, not assumed + * safe. The cost of a false refusal is a human clicking merge; the cost of a + * false allow is an unreviewed breaking change in someone's release path. + */ + +/** + * Parse a version out of a Dependabot token into [major, minor, patch]. + * Returns null when the token is not a version. + * + * Handles the shapes Dependabot actually writes: build metadata + * (`0.9.12+spec-1.1.0`), requirement operators (`>=1.2.0`), one-component + * versions (`4`), a leading `v`, and the full stop ending a `Bumps` line. + */ +export function parseVersion(token) { + if (typeof token !== 'string') return null; + const core = token + .trim() + .replace(/\.$/, '') + .replace(/^[<>=^~v\s]+/, '') + .split('+')[0] + .split('-')[0]; + if (!/^\d+(\.\d+){0,2}$/.test(core)) return null; + const parts = core.split('.').map(Number); + while (parts.length < 3) parts.push(0); + return parts; +} + +/** + * Is moving from `a` to `b` a breaking change? + * + * Uses the leftmost-non-zero rule, which is what Cargo and npm caret ranges + * mean by compatible: a major change is breaking; on a 0.x line a minor change + * is breaking; on a 0.0.x line a patch change is. Treating `0.7 -> 0.8` as a + * harmless minor would auto-merge exactly the Rust updates most likely to + * break a build. + */ +export function isBreakingUpdate(a, b) { + if (a[0] !== b[0]) return true; + if (a[0] === 0 && a[1] !== b[1]) return true; + if (a[0] === 0 && a[1] === 0 && a[2] !== b[2]) return true; + return false; +} + +// The three shapes Dependabot writes, each capturing the PACKAGE as well as +// the versions. The package is what makes two updates distinct: a grouped PR +// can move `wasmtime` and `wasmtime-wasi` from the same version to the same +// version, and keying on the versions alone collapses them into one. +const TITLE = /\b(?:bump|update)\s+(\S+?)(?:\s+requirement)?\s+from\s+(\S+)\s+to\s+(\S+)/i; +const UPDATES_LINE = /^Updates\s+`([^`]+)`\s+from\s+(\S+)\s+to\s+(\S+)/; +const BUMPS_LINE = /^Bumps\s+\[([^\]]+)\]\([^)]*\)\s+from\s+(\S+)\s+to\s+(\S+)/; + +/** + * Every update a Dependabot PR makes, as `{ name, from, to }`. + * + * Read from the title, and from the body ONLY on the summary lines Dependabot + * writes itself (`Updates \`x\` from ...`, `Bumps [x](...) from ...`). The body + * also quotes upstream release notes, which routinely say things like + * "migrate from 2.0.0 to 9.0.0"; reading those would invent updates. + * + * A grouped PR's title names no versions, so its body lines are the only + * source — and they must all be read, because a group's name says nothing + * about what is in it (witness#211, "the wasmtime group", carries 47 -> 48). + */ +export function extractUpdates(title, body) { + const updates = []; + const seen = new Set(); + const add = (name, from, to) => { + const clean = (v) => v.replace(/\.$/, ''); + const u = { name, from: clean(from), to: clean(to) }; + // A single-update PR states its update twice — in the title and in the + // body's `Bumps` line. De-duplicate on package AND versions, so that + // restatement collapses while two packages moving in step do not. + const key = `${u.name}|${u.from}|${u.to}`; + if (seen.has(key)) return; + seen.add(key); + updates.push(u); + }; + + const t = TITLE.exec(title || ''); + if (t) add(t[1], t[2], t[3]); + + for (const line of (body || '').split('\n')) { + const trimmed = line.trim(); + const m = UPDATES_LINE.exec(trimmed) || BUMPS_LINE.exec(trimmed); + if (m) add(m[1], m[2], m[3]); + } + return updates; +} + +/** + * The decision, with a reason a human can act on. + * + * @returns {{ allow: boolean, reason: string }} + */ +export function dependabotAutoMergeDecision(title, body) { + const updates = extractUpdates(title, body); + if (updates.length === 0) { + return { + allow: false, + reason: 'could not read any version update from the PR, so its semver impact is unknown' + }; + } + for (const { name, from, to } of updates) { + const a = parseVersion(from); + const b = parseVersion(to); + if (!a || !b) { + return { + allow: false, + reason: `could not read ${name} "${from}" -> "${to}" as semver, so its impact is unknown` + }; + } + if (isBreakingUpdate(a, b)) { + return { + allow: false, + reason: `breaking update ${name} ${from} -> ${to} needs a human review` + }; + } + } + return { allow: true, reason: `${updates.length} non-breaking update(s)` }; +} diff --git a/src/dependabot.js b/src/dependabot.js index 7bc89e6..a2e8d20 100644 --- a/src/dependabot.js +++ b/src/dependabot.js @@ -40,10 +40,48 @@ function extractLabelsFromConfig(dependabotConfig) { return [...labels]; } +const DEPENDABOT_PATH = '.github/dependabot.yml'; + +/** + * Does the repository already have a dependabot.yml? A 404 is the only answer + * that means no; any other failure is thrown, because it says nothing about + * whether the file exists. + */ +async function dependabotConfigExists(octokit, owner, repo) { + try { + await octokit.request('GET /repos/{owner}/{repo}/contents/{path}', { + owner, + repo, + path: DEPENDABOT_PATH + }); + return true; + } catch (error) { + if (error.status === 404) return false; + throw error; + } +} + +/** + * Write a dependabot.yml — but only where the repository has none. + * + * An existing file is the repository's own and is never overwritten, by any + * caller. It encodes what temper cannot know: which updates to group, which + * paths PR CI never exercises, which majors to hold (maintainer decision, + * temper#71). This is decided here, once, rather than at each caller, so that + * no path — /configure-repo, /generate-dependabot, the scheduler — can differ. + * + * @returns {Promise<{applied: boolean, reason: string}>} + */ async function applyDependabotConfig(octokit, owner, repo, dependabotConfig) { const config = getConfig(); try { + if (await dependabotConfigExists(octokit, owner, repo)) { + const reason = `${owner}/${repo} already has ${DEPENDABOT_PATH}; it is the repository's own and temper does not overwrite it`; + getLogger().info(reason); + return { applied: false, reason }; + } + getLogger().info(`Applying Dependabot configuration to ${owner}/${repo}`); const usePR = config.change_strategy?.use_pull_requests || false; @@ -73,6 +111,7 @@ async function applyDependabotConfig(octokit, owner, repo, dependabotConfig) { } getLogger().info(`✅ Applied Dependabot configuration to ${owner}/${repo}`); + return { applied: true, reason: `created ${DEPENDABOT_PATH}` }; } catch (error) { getLogger().error( `❌ Error applying Dependabot configuration to ${owner}/${repo}:`,