From 9d9bcc768802f0a6ca37db19851eb16c9c63c55e Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Wed, 28 Oct 2026 21:06:59 -0700 Subject: [PATCH 1/2] feat: per-package `requireBumpFile` for `bumpy check` --- .bumpy/require-bump-file.md | 5 + docs/cli.md | 2 +- docs/configuration.md | 56 ++++-- packages/bumpy/config-schema.json | 4 + packages/bumpy/src/commands/check.ts | 36 +++- packages/bumpy/src/commands/ci.ts | 141 ++++++++++++--- packages/bumpy/src/types.ts | 10 ++ .../test/core/check-require-bump-file.test.ts | 170 ++++++++++++++++++ skills/add-change/SKILL.md | 1 + 9 files changed, 374 insertions(+), 51 deletions(-) create mode 100644 .bumpy/require-bump-file.md create mode 100644 packages/bumpy/test/core/check-require-bump-file.test.ts diff --git a/.bumpy/require-bump-file.md b/.bumpy/require-bump-file.md new file mode 100644 index 0000000..aa8b752 --- /dev/null +++ b/.bumpy/require-bump-file.md @@ -0,0 +1,5 @@ +--- +'@varlock/bumpy': minor +--- + +Added `requireBumpFile: true` per-package config — `bumpy check` and `bumpy ci check` always fail when such a package changed but no bump file lists it, even if other bump files (or an empty bump file) are present. List it with type `none` to acknowledge a change without releasing. diff --git a/docs/cli.md b/docs/cli.md index 7dd6fa0..383f611 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -117,7 +117,7 @@ With `--snapshot `, publish derives a throwaway prerelease version per pen Verify that changed packages on the current branch have corresponding bump files. Compares your branch to the base branch, maps changed files to packages, and checks for matching bump files. -By default, exits non-zero only if **no** bump files exist at all (matching changesets behavior). Use `--strict` to require every changed package to be covered. +By default, exits non-zero only if **no** bump files exist at all (matching changesets behavior). Use `--strict` to require every changed package to be covered, or set [`requireBumpFile`](./configuration.md#requirebumpfile--packages-that-must-always-be-bumped) on specific packages. ```bash bumpy check diff --git a/docs/configuration.md b/docs/configuration.md index 91e0984..33f7912 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -178,22 +178,23 @@ Per-package settings can be defined in two places: Globs in per-package options (`changedFilePatterns`, `sourcePaths`) are always relative to the package's directory, wherever they're declared. -| Option | Type | Description | -| -------------------------- | -------------------------- | ---------------------------------------------------------------------------------------- | -| `managed` | `boolean` | Opt this package in or out of versioning | -| `directBump` | `boolean` | When `false`, the package only receives propagated bumps — never direct ones (see below) | -| `access` | `"public" \| "restricted"` | Override the global access level | -| `publishCommand` | `string \| string[]` | Custom command(s) to publish this package (replaces npm publish) | -| `buildCommand` | `string` | Command to run before publishing | -| `registry` | `string` | Custom npm registry URL | -| `skipNpmPublish` | `boolean` | Don't publish to npm (still creates git tags) | -| `checkPublished` | `string` | Custom command that outputs the currently published version | -| `changedFilePatterns` | `string[]` | Glob patterns for changed-file detection (replaces root setting, not merged) | -| `sourcePaths` | `string[]` | Globs for sources outside the package dir (see below) | -| `dependencyBumpRules` | `object` | Per-package override for dependency propagation rules | -| `cascadeTo` | `object` | Explicit cascade targets — glob pattern mapped to `{ trigger, bumpAs }` | -| `cascadeFrom` | `object` | Explicit cascade sources — glob pattern mapped to `{ trigger, bumpAs }` | -| `releaseTriggeringDevDeps` | `string[]` | devDependencies that affect published output — a change requires a release (see below) | +| Option | Type | Description | +| -------------------------- | -------------------------- | ---------------------------------------------------------------------------------------------- | +| `managed` | `boolean` | Opt this package in or out of versioning | +| `directBump` | `boolean` | When `false`, the package only receives propagated bumps — never direct ones (see below) | +| `requireBumpFile` | `boolean` | When `true`, `bumpy check` always fails if the package changed without a bump file (see below) | +| `access` | `"public" \| "restricted"` | Override the global access level | +| `publishCommand` | `string \| string[]` | Custom command(s) to publish this package (replaces npm publish) | +| `buildCommand` | `string` | Command to run before publishing | +| `registry` | `string` | Custom npm registry URL | +| `skipNpmPublish` | `boolean` | Don't publish to npm (still creates git tags) | +| `checkPublished` | `string` | Custom command that outputs the currently published version | +| `changedFilePatterns` | `string[]` | Glob patterns for changed-file detection (replaces root setting, not merged) | +| `sourcePaths` | `string[]` | Globs for sources outside the package dir (see below) | +| `dependencyBumpRules` | `object` | Per-package override for dependency propagation rules | +| `cascadeTo` | `object` | Explicit cascade targets — glob pattern mapped to `{ trigger, bumpAs }` | +| `cascadeFrom` | `object` | Explicit cascade sources — glob pattern mapped to `{ trigger, bumpAs }` | +| `releaseTriggeringDevDeps` | `string[]` | devDependencies that affect published output — a change requires a release (see below) | ### `sourcePaths` — sources outside the package directory @@ -236,6 +237,29 @@ Effects: - A bump file that directly names one (with a type other than `none`) is an error at plan time. - `bumpy check` treats changes in a `directBump: false` package as covered when a bump file covers any other member of its fixed group, and points there when one is missing. +### `requireBumpFile` — packages that must always be bumped + +By default `bumpy check` only fails when a branch has no bump file at all — a changed package missing from an otherwise-present bump file is just a warning. That's the right default for most packages, but some are only released when explicitly bumped (e.g. native helper binaries versioned independently of the package that uses them), so a forgotten bump silently skips a needed release. + +Mark those packages `requireBumpFile: true`: + +```jsonc +{ + "packages": { + "@mycli/native-helper-*": { "requireBumpFile": true }, + }, +} +``` + +When such a package changed on the branch, `bumpy check` and `bumpy ci check` fail unless a bump file lists it — regardless of other bump files on the branch. The PR comment shows it as blocking. + +- An empty bump file (`bumpy add --empty`) does **not** cover it. +- Listing it with type `none` does — use that to acknowledge a change that doesn't need a release. +- Propagated bumps (cascades, fixed/linked groups, dependency bumps) don't count; it must be listed explicitly. +- `--no-fail` still downgrades the failure to a warning. + +The error names each package and prints the command to fix it, e.g. `bumpy add --packages "@mycli/native-helper-darwin:patch"` (or `:none`). + ### Custom commands and `allowCustomCommands` The `publishCommand`, `buildCommand`, and `checkPublished` fields run shell commands during publishing. Because these execute with CI credentials, bumpy distinguishes between two trust levels: diff --git a/packages/bumpy/config-schema.json b/packages/bumpy/config-schema.json index 03af2af..4412bec 100644 --- a/packages/bumpy/config-schema.json +++ b/packages/bumpy/config-schema.json @@ -322,6 +322,10 @@ "type": "boolean", "description": "When false, this package can never be bumped directly by a bump file — it only receives propagated bumps (fixed/linked group, cascade, dependency). Use for derived artifacts like platform binary packages kept in a fixed group with their core package." }, + "requireBumpFile": { + "type": "boolean", + "description": "When true, `bumpy check` and `bumpy ci check` always fail if this package changed but no bump file lists it — regardless of other bump files on the branch. An empty bump file does not satisfy it; listing the package with type `none` does. Propagated bumps don't count." + }, "access": { "type": "string", "enum": ["public", "restricted"], diff --git a/packages/bumpy/src/commands/check.ts b/packages/bumpy/src/commands/check.ts index 974b23a..49da012 100644 --- a/packages/bumpy/src/commands/check.ts +++ b/packages/bumpy/src/commands/check.ts @@ -32,6 +32,7 @@ interface CheckOptions { * Designed for pre-push hooks — no GitHub API needed. * * Default: at least one bump file must exist, uncovered packages are warned. + * Packages with `requireBumpFile: true` must always be listed in a bump file. * --strict: every changed package must be covered. * --no-fail: warn only, never exit 1. * --hook pre-commit: only staged + committed bump files count. @@ -172,10 +173,13 @@ export async function checkCommand(rootDir: string, opts: CheckOptions = {}): Pr config, ); + // `requireBumpFile` packages must be listed in a bump file — nothing else covers them + const requiredMissing = missing.filter((name) => packages.get(name)?.bumpy?.requireBumpFile); + // An empty bump file covers all remaining packages (in non-strict mode) // It acts as a blanket acknowledgment that non-publishable changes are expected const hasAnyCoverage = effectiveBumpFiles.length > 0 || hasEmptyBumpFile; - const allCovered = missing.length === 0 || (hasEmptyBumpFile && !opts.strict); + const allCovered = missing.length === 0 || (hasEmptyBumpFile && !opts.strict && requiredMissing.length === 0); if (allCovered) { if (hasEmptyBumpFile && missing.length > 0) { @@ -197,11 +201,14 @@ export async function checkCommand(rootDir: string, opts: CheckOptions = {}): Pr // Determine failure behavior // - No coverage at all: fail by default (unless --no-fail) // - Partial coverage: fail only with --strict (unless --no-fail) - const willFail = !opts.noFail && (opts.strict || !hasAnyCoverage); - - (willFail ? log.error : log.warn)(`${missing.length} changed package(s) missing bump files:\n`); - for (const name of missing) { - const hint = hints.get(name); + // - A `requireBumpFile` package is missing: always fail (unless --no-fail) + const willFail = !opts.noFail && (opts.strict || !hasAnyCoverage || requiredMissing.length > 0); + + // With an empty bump file, only the required packages are still uncovered + const reported = hasEmptyBumpFile && !opts.strict ? requiredMissing : missing; + (willFail ? log.error : log.warn)(`${reported.length} changed package(s) missing bump files:\n`); + for (const name of reported) { + const hint = hints.get(name) ?? (requiredMissing.includes(name) ? 'requires a bump file (requireBumpFile)' : ''); console.log(` ${colorize(name, 'yellow')}${hint ? ` — ${hint}` : ''}`); } @@ -215,6 +222,11 @@ export async function checkCommand(rootDir: string, opts: CheckOptions = {}): Pr } console.log(); + if (requiredMissing.length > 0) { + log.dim('These packages require their own bump file — an empty bump file does not cover them. Run:'); + for (const line of requiredBumpFileCommands(requiredMissing)) log.dim(` ${line}`); + console.log(); + } if (opts.strict) { log.dim( "Run `bumpy add` to create a bump file. Use bump type `none` for packages that changed but don't need a bump.", @@ -246,6 +258,18 @@ function printBumpFileList( } } +/** + * Commands that satisfy `requireBumpFile` for the given packages — a patch bump, or a + * `none` entry to acknowledge the change without releasing. + */ +export function requiredBumpFileCommands(names: string[], runCmd = 'bumpy'): string[] { + const list = (type: string) => names.map((name) => `${name}:${type}`).join(','); + return [ + `${runCmd} add --packages "${list('patch')}" # release it`, + `${runCmd} add --packages "${list('none')}" # acknowledge without releasing`, + ]; +} + /** * Coverage adjustment for `directBump: false` packages. Such a package never gets its * own bump file — its changes ship by bumping another member of its fixed group. So a diff --git a/packages/bumpy/src/commands/ci.ts b/packages/bumpy/src/commands/ci.ts index 0d5dbad..777f37c 100644 --- a/packages/bumpy/src/commands/ci.ts +++ b/packages/bumpy/src/commands/ci.ts @@ -1,6 +1,6 @@ import { log, colorize } from '../utils/logger.ts'; import { getBumpyDir, loadConfig } from '../core/config.ts'; -import { findChangedPackages } from './check.ts'; +import { findChangedPackages, requiredBumpFileCommands } from './check.ts'; import { discoverWorkspace } from '../core/workspace.ts'; import { DependencyGraph } from '../core/dep-graph.ts'; import { readBumpFiles, filterBranchBumpFiles, recoverDeletedBumpFiles } from '../core/bump-file.ts'; @@ -28,7 +28,15 @@ import { createHash } from 'node:crypto'; import { appendFileSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; import { relative, resolve } from 'node:path'; import { resolveCommitMessage } from '../core/commit-message.ts'; -import type { BumpyConfig, BumpFile, PackageConfig, PackageManager, ReleasePlan, PlannedRelease } from '../types.ts'; +import type { + BumpyConfig, + BumpFile, + PackageConfig, + PackageManager, + ReleasePlan, + PlannedRelease, + WorkspacePackage, +} from '../types.ts'; // ---- PAT-scoped gh helpers ---- @@ -178,15 +186,24 @@ export async function ciCheckCommand(rootDir: string, opts: CheckOptions): Promi } if (prBumpFiles.length === 0) { - // An empty bump file signals intentionally no releases needed + // An empty bump file signals intentionally no releases needed — except for + // `requireBumpFile` packages, which must be listed in a bump file if (emptyBumpFileIds.length > 0 && parseErrors.length === 0) { - log.success('Empty bump file found — no releases needed.'); + const changedPackages = await findChangedPackages(changedFiles, packages, rootDir, config, compareBranch); + const requiredMissing = findRequiredMissing(changedPackages, new Set(), packages); + const willFail = requiredMissing.length > 0 && !opts.noFail; + if (requiredMissing.length > 0) { + logRequiredMissing(requiredMissing, willFail); + } else { + log.success('Empty bump file found — no releases needed.'); + } if (prNumber && (shouldComment || opts.emitComment)) { const prBranch = detectPrBranch(rootDir); - const body = formatEmptyBumpFileComment(emptyBumpFileIds, prNumber, prBranch); + const body = formatEmptyBumpFileComment(emptyBumpFileIds, prNumber, prBranch, requiredMissing, pm, willFail); if (opts.emitComment) writeCommentArtifact(opts.emitComment, body); if (shouldComment) await postOrUpdatePrComment(prNumber, body, rootDir); } + if (willFail) process.exit(1); return; } @@ -249,6 +266,21 @@ export async function ciCheckCommand(rootDir: string, opts: CheckOptions): Promi } } + // Check for uncovered packages + // Include both released packages and those explicitly listed in bump files (e.g. with 'none') + const listedPackages = new Set(); + for (const bf of prBumpFiles) { + for (const release of bf.releases) { + listedPackages.add(release.name); + } + } + const coveredPackages = new Set([...plan.releases.map((r) => r.name), ...listedPackages]); + const changedPackages = await findChangedPackages(changedFiles, packages, rootDir, config, compareBranch); + const missing = changedPackages.filter((name) => !coveredPackages.has(name)); + // `requireBumpFile` packages need an explicit entry — a propagated bump doesn't count + const requiredMissing = findRequiredMissing(changedPackages, listedPackages, packages); + const requiredWillFail = requiredMissing.length > 0 && !opts.noFail; + // Comment on PR if (prNumber && (shouldComment || opts.emitComment)) { const prBranch = detectPrBranch(rootDir); @@ -263,6 +295,8 @@ export async function ciCheckCommand(rootDir: string, opts: CheckOptions): Promi emptyBumpFileIds, prChannel, channels, + requiredMissing, + requiredWillFail, ); if (opts.emitComment) writeCommentArtifact(opts.emitComment, comment); if (shouldComment) await postOrUpdatePrComment(prNumber, comment, rootDir); @@ -273,22 +307,51 @@ export async function ciCheckCommand(rootDir: string, opts: CheckOptions): Promi process.exit(1); } - // Check for uncovered packages - // Include both released packages and those explicitly listed in bump files (e.g. with 'none') - const coveredPackages = new Set(plan.releases.map((r) => r.name)); - for (const bf of prBumpFiles) { - for (const release of bf.releases) { - coveredPackages.add(release.name); - } - } - const changedPackages = await findChangedPackages(changedFiles, packages, rootDir, config, compareBranch); - const missing = changedPackages.filter((name) => !coveredPackages.has(name)); - if (missing.length > 0) { - const willFail = opts.strict && !opts.noFail; - const logFn = willFail ? log.error : log.warn; - logFn(`${missing.length} changed package(s) not covered by bump files: ${missing.join(', ')}`); - if (willFail) process.exit(1); + const otherMissing = missing.filter((name) => !requiredMissing.includes(name)); + if (otherMissing.length > 0) { + const logFn = opts.strict && !opts.noFail ? log.error : log.warn; + logFn(`${otherMissing.length} changed package(s) not covered by bump files: ${otherMissing.join(', ')}`); } + if (requiredMissing.length > 0) logRequiredMissing(requiredMissing, requiredWillFail); + if (requiredWillFail || (missing.length > 0 && opts.strict && !opts.noFail)) process.exit(1); +} + +/** Changed packages with `requireBumpFile: true` that no bump file lists. */ +function findRequiredMissing( + changedPackages: string[], + listedPackages: Set, + packages: Map, +): string[] { + return changedPackages.filter((name) => packages.get(name)?.bumpy?.requireBumpFile && !listedPackages.has(name)); +} + +function logRequiredMissing(names: string[], willFail: boolean): void { + (willFail ? log.error : log.warn)( + `${names.length} changed package(s) require a bump file (requireBumpFile): ${names.join(', ')}`, + ); + for (const line of requiredBumpFileCommands(names)) log.dim(` ${line}`); +} + +/** + * PR comment section for `requireBumpFile` packages that no bump file lists. Shown as + * blocking unless the check runs with `--no-fail`. + */ +function formatRequiredMissingSection(names: string[], pm: PackageManager, willFail: boolean): string[] { + if (names.length === 0) return []; + const lines = [ + willFail ? '#### :x: Missing required bump files' : '#### ⚠️ Missing required bump files', + '', + `${names.length === 1 ? 'This package requires' : 'These packages require'} a bump file whenever ${names.length === 1 ? 'it changes' : 'they change'} ` + + `(\`requireBumpFile\`)${willFail ? ', so this check is failing' : ''}. An empty bump file does not cover ${names.length === 1 ? 'it' : 'them'}.`, + '', + ...names.map((name) => `- \`${name}\``), + '', + '```bash', + ...requiredBumpFileCommands(names, pmRunCommand(pm)), + '```', + '', + ]; + return lines; } // ---- ci deps ---- @@ -1466,6 +1529,8 @@ export function formatReleasePlanComment( emptyBumpFileIds: string[] = [], channel: ResolvedChannel | null = null, allChannels: Map | null = null, + requiredMissing: string[] = [], + requiredWillFail = true, ): string { const repo = process.env.GITHUB_REPOSITORY; const lines: string[] = []; @@ -1493,6 +1558,7 @@ export function formatReleasePlanComment( ].join('\n'); lines.push(preamble); lines.push(''); + lines.push(...formatRequiredMissingSection(requiredMissing, pm, requiredWillFail)); // Package list grouped by bump type const groups: Record = { major: [], minor: [], patch: [] }; @@ -1616,15 +1682,34 @@ function formatBumpFileErrorsComment(errors: string[], prBranch: string | null, return lines.join('\n'); } -function formatEmptyBumpFileComment(emptyBumpFileIds: string[], prNumber: string, prBranch: string | null): string { +export function formatEmptyBumpFileComment( + emptyBumpFileIds: string[], + prNumber: string, + prBranch: string | null, + requiredMissing: string[] = [], + pm: PackageManager = 'npm', + requiredWillFail = true, +): string { const repo = process.env.GITHUB_REPOSITORY; - const lines = [ - `bumpy-frog`, - '', - '**This PR includes an empty bump file — no version bump is needed.** :white_check_mark:', - '
', - '', - ]; + const lines = + requiredMissing.length > 0 + ? [ + `bumpy-frog`, + '', + '**This PR includes an empty bump file, but it changes packages that require their own bump file.**', + '
', + '', + ...formatRequiredMissingSection(requiredMissing, pm, requiredWillFail), + '#### Bump files in this PR', + '', + ] + : [ + `bumpy-frog`, + '', + '**This PR includes an empty bump file — no version bump is needed.** :white_check_mark:', + '
', + '', + ]; for (const id of emptyBumpFileIds) { const filename = `${id}.md`; diff --git a/packages/bumpy/src/types.ts b/packages/bumpy/src/types.ts index 5b7eb83..84bf90a 100644 --- a/packages/bumpy/src/types.ts +++ b/packages/bumpy/src/types.ts @@ -210,6 +210,16 @@ export interface PackageConfig { * member of its fixed group, and points there when one is missing */ directBump?: boolean; + /** + * When `true`, `bumpy check` / `bumpy ci check` always fail if this package changed + * but no bump file on the branch lists it — even when other bump files (or an empty + * bump file) are present. List it with type `none` to acknowledge a change without a + * release. Propagated bumps (cascades, fixed groups, dependency bumps) don't count. + * + * Use this for packages that are only released when explicitly bumped, where a + * forgotten bump file would silently skip a needed release. + */ + requireBumpFile?: boolean; access?: 'public' | 'restricted'; publishCommand?: string | string[]; buildCommand?: string; diff --git a/packages/bumpy/test/core/check-require-bump-file.test.ts b/packages/bumpy/test/core/check-require-bump-file.test.ts new file mode 100644 index 0000000..758dacc --- /dev/null +++ b/packages/bumpy/test/core/check-require-bump-file.test.ts @@ -0,0 +1,170 @@ +import { test, expect, describe, beforeEach, afterEach, spyOn } from 'bun:test'; +import { resolve } from 'node:path'; +import { mkdir, writeFile, mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { checkCommand, requiredBumpFileCommands } from '../../src/commands/check.ts'; +import { ciCheckCommand, formatEmptyBumpFileComment, formatReleasePlanComment } from '../../src/commands/ci.ts'; +import { createTempGitRepo, cleanupTempDir, gitInDir, makeBumpFile, makeReleasePlan } from '../helpers.ts'; + +/** + * Integration tests for `requireBumpFile`: a temp monorepo where `helper` requires its + * own bump file and `app` uses the lenient default. Each test changes both packages on + * a feature branch, adds some bump files, and asserts whether the check exits 1. + */ +describe('requireBumpFile', () => { + let tmpDir: string; + let remote: string; + let output: string[]; + const spies: Array<{ mockRestore(): void }> = []; + + async function writeAt(path: string, content: string): Promise { + await mkdir(resolve(tmpDir, path, '..'), { recursive: true }); + await writeFile(resolve(tmpDir, path), content); + } + + beforeEach(async () => { + tmpDir = await createTempGitRepo(); + gitInDir(['branch', '-M', 'main'], tmpDir); + gitInDir(['config', 'user.email', 'test@example.com'], tmpDir); + gitInDir(['config', 'user.name', 'Test'], tmpDir); + + await writeAt('package.json', JSON.stringify({ name: 'root', private: true, workspaces: ['packages/*'] })); + await writeAt('packages/app/package.json', JSON.stringify({ name: 'app', version: '1.0.0' })); + await writeAt('packages/app/index.js', 'export {};\n'); + await writeAt('packages/helper/package.json', JSON.stringify({ name: 'helper', version: '1.0.0' })); + await writeAt('packages/helper/index.js', 'export {};\n'); + await writeAt('.bumpy/_config.json', JSON.stringify({ packages: { 'help*': { requireBumpFile: true } } })); + gitInDir(['add', '.'], tmpDir); + gitInDir(['commit', '-m', 'base'], tmpDir); + + remote = await mkdtemp(resolve(tmpdir(), 'bumpy-remote-')); + gitInDir(['init', '--bare'], remote); + gitInDir(['remote', 'add', 'origin', remote], tmpDir); + gitInDir(['push', '-u', 'origin', 'main'], tmpDir); + + gitInDir(['checkout', '-b', 'feature'], tmpDir); + await writeAt('packages/app/index.js', 'export const a = 1;\n'); + await writeAt('packages/helper/index.js', 'export const h = 1;\n'); + + output = []; + const capture = (...args: unknown[]) => void output.push(args.join(' ')); + spies.push(spyOn(console, 'log').mockImplementation(capture)); + spies.push(spyOn(console, 'error').mockImplementation(capture)); + spies.push( + spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as typeof process.exit), + ); + }); + + afterEach(async () => { + for (const spy of spies) spy.mockRestore(); + spies.length = 0; + await cleanupTempDir(tmpDir); + await rm(remote, { recursive: true, force: true }); + }); + + async function commitBumpFiles(files: Record): Promise { + for (const [id, content] of Object.entries(files)) await writeAt(`.bumpy/${id}.md`, content); + gitInDir(['add', '.'], tmpDir); + gitInDir(['commit', '-m', 'change'], tmpDir); + } + + /** Run a check; returns the exit code (0 when it returned normally) */ + async function run(fn: () => Promise): Promise { + try { + await fn(); + return 0; + } catch (err) { + const match = /process\.exit\((\d*)\)/.exec((err as Error).message); + if (!match) throw err; + return Number(match[1] || 0); + } + } + + describe('bumpy check', () => { + test('fails when a required package is uncovered, even with another bump file', async () => { + await commitBumpFiles({ 'app-change': "---\n'app': patch\n---\n\nFix app\n" }); + expect(await run(() => checkCommand(tmpDir))).toBe(1); + const text = output.join('\n'); + expect(text).toContain('helper'); + expect(text).toContain('add --packages "helper:patch"'); + expect(text).toContain('add --packages "helper:none"'); + }); + + test('passes when the required package is listed with type none', async () => { + await commitBumpFiles({ 'app-change': "---\n'app': patch\n'helper': none\n---\n\nFix app\n" }); + expect(await run(() => checkCommand(tmpDir))).toBe(0); + }); + + test('an empty bump file does not satisfy a required package', async () => { + await commitBumpFiles({ nothing: '---\n---\n' }); + expect(await run(() => checkCommand(tmpDir))).toBe(1); + const text = output.join('\n'); + expect(text).toContain('helper'); + // app is still acknowledged by the empty bump file + expect(text).not.toMatch(/\bapp\b.*—/); + }); + + test('non-required packages keep the lenient default', async () => { + await commitBumpFiles({ 'helper-change': "---\n'helper': patch\n---\n\nFix helper\n" }); + expect(await run(() => checkCommand(tmpDir))).toBe(0); + expect(output.join('\n')).toContain('app'); + }); + + test('--no-fail only warns', async () => { + await commitBumpFiles({ 'app-change': "---\n'app': patch\n---\n\nFix app\n" }); + expect(await run(() => checkCommand(tmpDir, { noFail: true }))).toBe(0); + }); + }); + + describe('bumpy ci check', () => { + test('fails when a required package is uncovered, even with another bump file', async () => { + await commitBumpFiles({ 'app-change': "---\n'app': patch\n---\n\nFix app\n" }); + expect(await run(() => ciCheckCommand(tmpDir, { comment: false }))).toBe(1); + }); + + test('an empty bump file does not satisfy a required package', async () => { + await commitBumpFiles({ nothing: '---\n---\n' }); + expect(await run(() => ciCheckCommand(tmpDir, { comment: false }))).toBe(1); + }); + + test('passes when the required package is listed with type none', async () => { + await commitBumpFiles({ 'app-change': "---\n'app': patch\n'helper': none\n---\n\nFix app\n" }); + expect(await run(() => ciCheckCommand(tmpDir, { comment: false }))).toBe(0); + }); + + test('--no-fail only warns', async () => { + await commitBumpFiles({ nothing: '---\n---\n' }); + expect(await run(() => ciCheckCommand(tmpDir, { comment: false, noFail: true }))).toBe(0); + }); + }); +}); + +describe('requireBumpFile PR comments', () => { + test('release plan comment shows missing required packages as blocking', () => { + const bf = makeBumpFile('app-change', [{ name: 'app', type: 'patch' }]); + const body = formatReleasePlanComment(makeReleasePlan([], [bf]), [bf], '1', null, 'pnpm', [], [], [], null, null, [ + 'helper', + ]); + expect(body).toContain(':x: Missing required bump files'); + expect(body).toContain('this check is failing'); + expect(body).toContain('pnpm exec bumpy add --packages "helper:patch"'); + }); + + test('empty bump file comment is not a green light when required packages are missing', () => { + const body = formatEmptyBumpFileComment(['nothing'], '1', null, ['helper'], 'npm'); + expect(body).not.toContain('no version bump is needed'); + expect(body).toContain('npx bumpy add --packages "helper:none"'); + }); + + test('--no-fail renders the section as a warning', () => { + const body = formatEmptyBumpFileComment(['nothing'], '1', null, ['helper'], 'npm', false); + expect(body).toContain('⚠️ Missing required bump files'); + expect(body).not.toContain('this check is failing'); + }); + + test('commands list every package', () => { + expect(requiredBumpFileCommands(['@s/a', '@s/b'])[0]).toContain('bumpy add --packages "@s/a:patch,@s/b:patch"'); + }); +}); diff --git a/skills/add-change/SKILL.md b/skills/add-change/SKILL.md index 4a076ad..bf28417 100644 --- a/skills/add-change/SKILL.md +++ b/skills/add-change/SKILL.md @@ -125,6 +125,7 @@ It's a file-level flag (a bump file has one shared body), so it applies to every - Only include packages that have **actual code changes** — bumpy handles dependency propagation automatically - Never list a package configured with `directBump: false` (check `.bumpy/_config.json` `packages` and each `package.json` `"bumpy"` field) — such packages only receive propagated bumps; bump the package that drives them (e.g. their fixed-group member) instead - If the user hasn't made any changes yet, ask what they're planning to change +- Always list changed packages configured with `requireBumpFile: true` — an empty bump file doesn't cover them; use type `none` if they don't need a release - If the change doesn't affect any publishable packages (e.g., only root config files), suggest using `bumpy add --empty` to satisfy CI checks - One bump file per logical change — don't combine unrelated changes - **Keep bump files up to date** — as work continues on a branch, the bump file should reflect the final state of all changes, not just the first commit. If packages were added/removed or the bump level changed (e.g., a patch fix grew into a minor feature), update the existing bump file accordingly From eaba8e69770a83fde08944e8c4469f0d270c436f Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Wed, 28 Oct 2026 21:19:05 -0700 Subject: [PATCH 2/2] fix: requireBumpFile ignores fixed-group coverage and shows in no-bump-file comments --- packages/bumpy/src/commands/check.ts | 8 +++-- packages/bumpy/src/commands/ci.ts | 28 +++++++++++++++-- .../test/core/check-require-bump-file.test.ts | 30 ++++++++++++++++++- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/packages/bumpy/src/commands/check.ts b/packages/bumpy/src/commands/check.ts index 49da012..ec6afc2 100644 --- a/packages/bumpy/src/commands/check.ts +++ b/packages/bumpy/src/commands/check.ts @@ -173,8 +173,12 @@ export async function checkCommand(rootDir: string, opts: CheckOptions = {}): Pr config, ); - // `requireBumpFile` packages must be listed in a bump file — nothing else covers them - const requiredMissing = missing.filter((name) => packages.get(name)?.bumpy?.requireBumpFile); + // `requireBumpFile` packages must be listed in a bump file — nothing else covers them, + // including a fixed-group member's bump for a directBump: false package + const requiredMissing = changedPackages.filter( + (name) => packages.get(name)?.bumpy?.requireBumpFile && !coveredPackages.has(name), + ); + for (const name of requiredMissing) if (!missing.includes(name)) missing.push(name); // An empty bump file covers all remaining packages (in non-strict mode) // It acts as a blanket acknowledgment that non-publishable changes are expected diff --git a/packages/bumpy/src/commands/ci.ts b/packages/bumpy/src/commands/ci.ts index 777f37c..ee44945 100644 --- a/packages/bumpy/src/commands/ci.ts +++ b/packages/bumpy/src/commands/ci.ts @@ -214,6 +214,7 @@ export async function ciCheckCommand(rootDir: string, opts: CheckOptions): Promi return; } + const requiredMissing = findRequiredMissing(changedPackages, new Set(), packages); const willFail = !opts.noFail || parseErrors.length > 0; const msg = parseErrors.length > 0 @@ -222,7 +223,9 @@ export async function ciCheckCommand(rootDir: string, opts: CheckOptions): Promi if (willFail) log.error(msg); else log.warn(msg); // Point at the empty-bump-file escape hatch so the CLI matches the PR comment. - if (parseErrors.length === 0 && willFail) { + if (parseErrors.length === 0 && requiredMissing.length > 0) { + logRequiredMissing(requiredMissing, willFail); + } else if (parseErrors.length === 0 && willFail) { log.dim('Run `bumpy add` to declare a release, or `bumpy add --empty` to acknowledge that no release is needed.'); } @@ -231,7 +234,7 @@ export async function ciCheckCommand(rootDir: string, opts: CheckOptions): Promi const body = parseErrors.length > 0 ? formatBumpFileErrorsComment(parseErrors, prBranch, pm) - : formatNoBumpFilesComment(prBranch, pm, willFail, changedPackages); + : formatNoBumpFilesComment(prBranch, pm, willFail, changedPackages, requiredMissing); if (opts.emitComment) writeCommentArtifact(opts.emitComment, body); if (shouldComment) await postOrUpdatePrComment(prNumber, body, rootDir); } @@ -1744,11 +1747,27 @@ export function formatNoBumpFilesComment( pm: PackageManager, willFail = false, changedPackages: string[] = [], + requiredMissing: string[] = [], ): string { const runCmd = pmRunCommand(pm); const addLink = buildAddBumpFileLink(prBranch); const emptyLink = buildAddEmptyBumpFileLink(prBranch); + // `--no-fail` with required packages changed — don't say "you're good to go" + if (!willFail && requiredMissing.length > 0) { + const lines = [ + `bumpy-frog`, + '', + '**This PR has no bump file, but it changes packages that require their own bump file.**', + '
\n', + ...formatRequiredMissingSection(requiredMissing, pm, false), + ]; + if (addLink) lines.push(`Or [click here to add a bump file](${addLink}) directly on GitHub.`); + lines.push('\n---'); + lines.push(`_This comment is maintained by [bumpy](https://bumpy.varlock.dev)._`); + return lines.join('\n'); + } + if (!willFail) { const lines = [ `bumpy-frog`, @@ -1789,10 +1808,13 @@ export function formatNoBumpFilesComment( lines.push(''); } + lines.push(...formatRequiredMissingSection(requiredMissing, pm, true)); lines.push( '**If these changes should be released**, add a bump file describing the version bump. ' + '**If no release is needed** (e.g. a dependency-only or dev-only change), add an _empty_ ' + - 'bump file to acknowledge that intent — that satisfies this check without bumping any package.\n', + 'bump file to acknowledge that intent — that satisfies this check without bumping any package' + + (requiredMissing.length > 0 ? ' other than the required ones above' : '') + + '.\n', ); lines.push('```bash'); lines.push(`${runCmd} add # describe a release`); diff --git a/packages/bumpy/test/core/check-require-bump-file.test.ts b/packages/bumpy/test/core/check-require-bump-file.test.ts index 758dacc..7defce0 100644 --- a/packages/bumpy/test/core/check-require-bump-file.test.ts +++ b/packages/bumpy/test/core/check-require-bump-file.test.ts @@ -3,7 +3,12 @@ import { resolve } from 'node:path'; import { mkdir, writeFile, mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { checkCommand, requiredBumpFileCommands } from '../../src/commands/check.ts'; -import { ciCheckCommand, formatEmptyBumpFileComment, formatReleasePlanComment } from '../../src/commands/ci.ts'; +import { + ciCheckCommand, + formatEmptyBumpFileComment, + formatNoBumpFilesComment, + formatReleasePlanComment, +} from '../../src/commands/ci.ts'; import { createTempGitRepo, cleanupTempDir, gitInDir, makeBumpFile, makeReleasePlan } from '../helpers.ts'; /** @@ -116,6 +121,19 @@ describe('requireBumpFile', () => { await commitBumpFiles({ 'app-change': "---\n'app': patch\n---\n\nFix app\n" }); expect(await run(() => checkCommand(tmpDir, { noFail: true }))).toBe(0); }); + + test('a fixed-group bump does not cover a required directBump: false package', async () => { + await writeAt( + '.bumpy/_config.json', + JSON.stringify({ + fixed: [['app', 'helper']], + packages: { helper: { requireBumpFile: true, directBump: false } }, + }), + ); + await commitBumpFiles({ 'app-change': "---\n'app': patch\n---\n\nFix app\n" }); + expect(await run(() => checkCommand(tmpDir))).toBe(1); + expect(output.join('\n')).toContain('helper:none'); + }); }); describe('bumpy ci check', () => { @@ -164,6 +182,16 @@ describe('requireBumpFile PR comments', () => { expect(body).not.toContain('this check is failing'); }); + test('no-bump-file comment does not offer an empty bump file as sufficient for required packages', () => { + const failing = formatNoBumpFilesComment(null, 'npm', true, ['app', 'helper'], ['helper']); + expect(failing).toContain(':x: Missing required bump files'); + expect(failing).toContain('other than the required ones above'); + const noFail = formatNoBumpFilesComment(null, 'npm', false, ['app', 'helper'], ['helper']); + expect(noFail).not.toContain("you're good to go"); + expect(noFail).toContain('⚠️ Missing required bump files'); + expect(noFail).toContain('npx bumpy add --packages "helper:none"'); + }); + test('commands list every package', () => { expect(requiredBumpFileCommands(['@s/a', '@s/b'])[0]).toContain('bumpy add --packages "@s/a:patch,@s/b:patch"'); });