From f37162d19e12542c2ce08d766fc20619cd9b0572 Mon Sep 17 00:00:00 2001 From: Taylor Buchanan Date: Sat, 22 Aug 2026 00:36:56 -0500 Subject: [PATCH 1/5] Add catalog-aware changeset gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `changeset status` maps changed files to packages, but a pnpm catalog moves every dependency range to `pnpm-workspace.yaml` at the workspace root, which belongs to no package. A catalog bump therefore reads as "no package changed", while pnpm rewrites `catalog:` to the concrete range at pack time — so every consumer's published manifest moves with no version bump behind it, and the new range sits unreleased until some unrelated PR happens to bump the package. `gtb changeset check` diffs the catalog blocks between a base ref and HEAD, maps each changed entry to the published packages declaring it in a runtime dependency field, and fails when no changeset covers them. Both revisions are compared as maps, so reformatting reports nothing. What keeps it quiet: only runtime fields count, so publishing stripping devDependencies means a test-only bump never fires; bundled deps, private packages, and config `ignore` entries are excluded. An empty changeset is not coverage — it is the documented way to say "no release", which is the claim the gate exists to challenge. `changeset-check.yml` runs it beside the existing gate, behind the same `gtb-from-source` input `cd.yml` uses. The job now installs for the gtb bin, so the stock check moves from `pnpm dlx` to `pnpm exec`: the `pnpm-resolve-pinned` indirection existed only to skip the install, and it already required `@changesets/cli` as a root devDependency to read the version from the lockfile. Base-ref resolution is left as-is; see #425. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/catalog-changeset-gate.md | 27 ++ .github/workflows/changeset-check.yml | 33 +- .github/workflows/pr.yml | 4 + AGENTS.md | 13 +- .../cli/skills/gtb-build-pipeline/SKILL.md | 14 +- packages/cli/src/commands/index.ts | 2 + packages/cli/src/commands/root/changeset.ts | 269 +++++++++++++++ packages/cli/src/commands/root/index.ts | 1 + packages/cli/src/commands/root/names.ts | 1 + packages/cli/src/lib/catalog-gate.ts | 202 +++++++++++ packages/cli/src/lib/discovery.ts | 56 +++ packages/cli/test/catalog-gate.test.ts | 326 ++++++++++++++++++ packages/cli/test/changeset-check.test.ts | 168 +++++++++ .../cli/test/coverage-codecov-upload.test.ts | 1 + packages/cli/test/turbo-config.helpers.ts | 2 + 15 files changed, 1109 insertions(+), 10 deletions(-) create mode 100644 .changeset/catalog-changeset-gate.md create mode 100644 packages/cli/src/commands/root/changeset.ts create mode 100644 packages/cli/src/lib/catalog-gate.ts create mode 100644 packages/cli/test/catalog-gate.test.ts create mode 100644 packages/cli/test/changeset-check.test.ts diff --git a/.changeset/catalog-changeset-gate.md b/.changeset/catalog-changeset-gate.md new file mode 100644 index 00000000..cfb8e603 --- /dev/null +++ b/.changeset/catalog-changeset-gate.md @@ -0,0 +1,27 @@ +--- +'@gtbuchanan/cli': minor +--- + +Add `gtb changeset check`, a catalog-aware changeset gate. + +`changeset status` maps changed files to packages, but a pnpm catalog moves +every dependency range to `pnpm-workspace.yaml` at the workspace root, which +belongs to no package. A catalog bump therefore reads as "no package changed", +while pnpm rewrites `catalog:` to the concrete range at pack time — so every +consumer's published manifest moves with no version bump behind it, and the new +range sits unreleased until some unrelated PR happens to bump the package. + +The gate diffs the `catalog:` / `catalogs:` blocks between a base ref and HEAD, +maps each changed entry to the published packages declaring it in a runtime +dependency field, and fails when no changeset covers them. `devDependencies`, +bundled dependencies, private packages, and anything in the changesets config's +`ignore` are excluded, and an empty changeset does not count as coverage. + +`changeset-check.yml` runs it alongside the existing `changeset status` gate, +behind the same `gtb-from-source` input `cd.yml` uses. Since the job now +installs for the gtb bin, the stock check moved from `pnpm dlx` to +`pnpm exec` — the `pnpm-resolve-pinned` indirection existed only to avoid the +install, and it already required `@changesets/cli` to be a root devDependency +to resolve the version from the lockfile. + +`PackageCapabilities` gains a `catalogDependencies` field. diff --git a/.github/workflows/changeset-check.yml b/.github/workflows/changeset-check.yml index 32832f5d..4a4e94cf 100644 --- a/.github/workflows/changeset-check.yml +++ b/.github/workflows/changeset-check.yml @@ -14,18 +14,39 @@ jobs: - uses: gtbuchanan/tooling/.github/actions/mise-setup@main - - id: changesets - uses: gtbuchanan/tooling/.github/actions/pnpm-resolve-pinned@main - with: - package: '@changesets/cli' + # Both gates run out of node_modules: `changeset` for the stock check, + # the gtb bin for the catalog gate. + - uses: gtbuchanan/tooling/.github/actions/pnpm-tasks@main - name: Verify changeset exists - run: pnpm dlx @changesets/cli@${{ steps.changesets.outputs.version }} status --since=origin/main + run: pnpm exec changeset status --since=origin/main + + # `changeset status` maps changed *files* to packages, so a pnpm catalog + # bump — which edits only the workspace root — reads as "no package + # changed", even though pnpm rewrites `catalog:` to the concrete range at + # pack time and every consumer's published manifest moves. No-ops in a + # repo with no catalog. + - name: Verify catalog changes are released + run: >- + ${{ inputs.gtb-from-source && 'pnpm run gtb' || 'pnpm exec gtb' }} + changeset check --since=origin/main name: Changeset +# The catalog gate runs through `gtb`, so the caller must depend on +# `@gtbuchanan/cli` (the same requirement `cd.yml` already carries). on: - workflow_call: {} + workflow_call: + inputs: + gtb-from-source: + default: false + description: >- + Run gtb from the workspace source (`pnpm run gtb`) instead of the + installed bin (`pnpm exec gtb`). Set true only by the repo that + vendors `@gtbuchanan/cli` as a workspace package (tooling itself), + whose bin is not built in this job. Consumers leave it false and + run their installed, prebuilt bin. + type: boolean permissions: contents: read diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index b2748e7c..6389ead3 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -3,6 +3,10 @@ jobs: changeset: name: Changeset uses: ./.github/workflows/changeset-check.yml + # This repo vendors @gtbuchanan/cli as a workspace package, so the + # catalog gate runs gtb from source rather than an installed bin. + with: + gtb-from-source: true ci: name: CI diff --git a/AGENTS.md b/AGENTS.md index 16ea55b2..71cf64ad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ mise.toml — Pin dev-tool versions for local + CI; postinstall hoo renovate.json — Repo-local Renovate config (extends the shared preset) workflows/ cd.yml — Reusable: gtb version (changesets + manifest sync) then gtb publish (npm via OIDC + non-npm channels) - changeset-check.yml — Reusable: verify a changeset exists + changeset-check.yml — Reusable: verify a changeset exists, and that catalog changes reaching published packages are released ci.yml — Reusable: build + slow + e2e + coverage dependency-review.yml — Reusable: scan dep changes (vulns + licenses) pr.yml — Pipeline (PR): ci + changeset + deps + pre-commit @@ -367,8 +367,15 @@ through `package.json` scripts backed by `gtb` leaf commands. `true` in `release.yml` to run gtb from source (`pnpm run gtb`), whose bin these jobs don't build. This mirrors the `gtbPrefix` repo-shape resolution used for generated scripts and mise tasks. -- **`changeset-check.yml`** — Verifies a changeset exists on every PR. - Use `pnpm changeset --empty` for PRs that don't need a version bump. +- **`changeset-check.yml`** — Two PR gates on releasability. + `changeset status` verifies a changeset exists; use `pnpm changeset --empty` + for PRs that don't need a version bump. `gtb changeset check` then fails a + catalog change that reaches a published package with no changeset releasing + it — a gap `status` cannot see, since a catalog bump edits only the + workspace root; see the `gtb-build-pipeline` skill. The job installs so both + run from `node_modules`. The gate runs through `gtb`, so the caller must + depend on `@gtbuchanan/cli`; the `gtb-from-source` input (default `false`) + flips the invocation exactly as in `cd.yml`. - **`dependency-review.yml`** — Two PR gates on newly-changed deps. `Dependency Review` runs `actions/dependency-review-action` (fails on advisories at `fail-on-severity`, default `moderate`, and on diff --git a/packages/cli/skills/gtb-build-pipeline/SKILL.md b/packages/cli/skills/gtb-build-pipeline/SKILL.md index 7d6f27c6..7cb25536 100644 --- a/packages/cli/skills/gtb-build-pipeline/SKILL.md +++ b/packages/cli/skills/gtb-build-pipeline/SKILL.md @@ -1,6 +1,6 @@ --- name: gtb-build-pipeline -description: Build pipeline guidance for projects using @gtbuchanan/cli. Covers the Turborepo task graph, gtb sync and verify (including scoped runs), the gtb hk pre-commit runner, the gtb turbo wrapper (with the Android/Termux escape hatch), consumer script customization, and test-bucket strategy. Trigger keywords - @gtbuchanan/cli, @gtbuchanan/pnpm-termux-shim, turbo.json, gtb sync, gtb sync mise, gtb verify, gtb verify mise, gtb turbo, gtb task, gtb hk, hk:all, hk:base, mise.tasks.toml, compile:ts, pack:npm, deploy:skills, task graph, transit. +description: Build pipeline guidance for projects using @gtbuchanan/cli. Covers the Turborepo task graph, gtb sync and verify (including scoped runs), the gtb hk pre-commit runner, the gtb changeset catalog gate, the gtb turbo wrapper (with the Android/Termux escape hatch), consumer script customization, and test-bucket strategy. Trigger keywords - @gtbuchanan/cli, @gtbuchanan/pnpm-termux-shim, turbo.json, gtb sync, gtb sync mise, gtb verify, gtb verify mise, gtb turbo, gtb task, gtb hk, gtb changeset check, hk:all, hk:base, mise.tasks.toml, pnpm catalog, compile:ts, pack:npm, deploy:skills, task graph, transit. --- # @gtbuchanan/cli build pipeline @@ -201,6 +201,18 @@ An e2e suite won't catch this on your behalf. A harness that installs each works Invoked via mise (`mise run hk:base`) so hk and its tools resolve from mise. The mise task resolves `gtb` itself per repo shape — see the `mise.tasks.toml` notes above. +## Catalog changeset gate (`gtb changeset check`) + +`gtb changeset check` fails when a pnpm catalog change reaches a published package with no changeset releasing it. It exists because `changeset status` maps changed **files** to packages, and a catalog moves every dependency range to `pnpm-workspace.yaml` at the workspace root — which belongs to no package. The bump therefore reads as "no package changed", while pnpm rewrites `catalog:` to the concrete range at pack time, so every consumer's published manifest moves with no version bump behind it. + +- `gtb changeset check [--since ] [--ignore ]` — diffs the `catalog:` / `catalogs:` blocks between the base ref (default `origin/main`) and HEAD, maps each changed entry to the published packages declaring it, and exits non-zero for any that no changeset covers. + +Both revisions are parsed and compared as maps, so reordering or reformatting `pnpm-workspace.yaml` reports nothing. Only runtime fields count (`dependencies`, `peerDependencies`, `optionalDependencies`) — publishing strips `devDependencies`, so a test-only bump never fires — and bundled dependencies are excluded, matching how `workspaceDependencies` is collected. Private packages and anything in the changesets config's `ignore` are skipped. An empty changeset is **not** coverage: it is the documented way to say "no release", which is the claim the gate exists to challenge. Removed catalog entries are skipped, since orphaning one requires a `package.json` edit that `changeset status` already sees. + +The gate is quiet in practice because of Renovate's `rangeStrategy`: under npm's default an in-range bump touches only `pnpm-lock.yaml`, and the catalog is edited only when the new version falls outside the declared range — so a catalog edit is already a range-boundary crossing, exactly the consumer-visible set. Switching to `rangeStrategy: bump` would make every patch update rewrite the catalog and turn the gate into noise. + +A repo with no catalog no-ops. + ## Android-Termux setup Two issues are caused by Termux's Node reporting `process.platform === 'android'`; a third (memory pressure) is unrelated and applies to any low-memory host. Native Android support upstream was declined in [vercel/turborepo#5616](https://github.com/vercel/turborepo/issues/5616), so `gtb turbo` ships the workaround instead. diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index f770a4a8..f92c6155 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -1,4 +1,5 @@ import { defineCommand } from 'citty'; +import { changeset } from './root/changeset.ts'; import { hk } from './root/hk.ts'; import { rootNames } from './root/names.ts'; import { prepare } from './root/prepare.ts'; @@ -19,6 +20,7 @@ export const main = defineCommand({ name: 'gtb', }, subCommands: { + [rootNames.changeset]: changeset, [rootNames.hk]: hk, [rootNames.prepare]: prepare, [rootNames.publish]: publish, diff --git a/packages/cli/src/commands/root/changeset.ts b/packages/cli/src/commands/root/changeset.ts new file mode 100644 index 00000000..cbaafac7 --- /dev/null +++ b/packages/cli/src/commands/root/changeset.ts @@ -0,0 +1,269 @@ +import { readFileSync, readdirSync } from 'node:fs'; +import path from 'node:path'; +import { defineCommand } from 'citty'; +import * as v from 'valibot'; +import { + type CatalogConsumer, + diffCatalogs, + findUncoveredCatalogChanges, + formatCatalogFinding, + parseCatalogs, + parseChangesetPackages, +} from '../../lib/catalog-gate.ts'; +import { discoverWorkspace } from '../../lib/discovery.ts'; +import { readJsonFile } from '../../lib/file-writer.ts'; +import { type Logger, createLogger } from '../../lib/logger.ts'; +import { type ExecResult, execute } from '../../lib/process.ts'; +import { StringArray } from '../../lib/schemas.ts'; +import { rootNames } from './names.ts'; +import { parseIgnoreArgs } from './verify.ts'; + +const workspaceFileName = 'pnpm-workspace.yaml'; +const changesetDirName = '.changeset'; +const defaultBaseRef = 'origin/main'; + +/** + * Everything {@link checkCatalogGate} needs, already read from disk and git. + */ +export interface CheckCatalogGateInputs { + /** + * `pnpm-workspace.yaml` as of the base ref, or `''` when the base predates + * the file. + */ + readonly baseWorkspace: string; + /** + * Raw contents of every pending changeset. + */ + readonly changesetSources: readonly string[]; + readonly headWorkspace: string; + readonly ignored: ReadonlySet; + readonly packages: readonly CatalogConsumer[]; +} + +/** + * The pure core of the gate: reports one drift line per published package that + * a catalog change reaches with no changeset releasing it. An empty result + * means no drift. + */ +export const checkCatalogGate = ( + inputs: CheckCatalogGateInputs, +): readonly string[] => { + const changes = diffCatalogs( + parseCatalogs(inputs.baseWorkspace), + parseCatalogs(inputs.headWorkspace), + ); + if (changes.length === 0) { + return []; + } + const covered = new Set(inputs.changesetSources.flatMap(parseChangesetPackages)); + + return findUncoveredCatalogChanges({ + changes, + covered, + ignored: inputs.ignored, + packages: inputs.packages, + }).map(formatCatalogFinding); +}; + +const ChangesetConfigSchema = v.looseObject({ + ignore: v.optional(StringArray), +}); + +/** + * Package names the changesets config already excludes from releases. They + * can't be covered by a changeset, so the gate must not demand one. + */ +const readConfiguredIgnores = (rootDir: string): readonly string[] => { + try { + const raw = readJsonFile(path.join(rootDir, changesetDirName, 'config.json')); + const result = v.safeParse(ChangesetConfigSchema, raw); + + return result.success ? result.output.ignore ?? [] : []; + } catch { + return []; + } +}; + +const readChangesetSources = (rootDir: string): readonly string[] => { + const dir = path.join(rootDir, changesetDirName); + let entries: readonly string[]; + try { + entries = readdirSync(dir); + } catch { + return []; + } + + return entries + .filter(name => name.endsWith('.md') && name.toLowerCase() !== 'readme.md') + .map(name => readFileSync(path.join(dir, name), 'utf8')); +}; + +const readHeadWorkspace = (rootDir: string): string => { + try { + return readFileSync(path.join(rootDir, workspaceFileName), 'utf8'); + } catch { + return ''; + } +}; + +/** + * Side-effecting git access, injected so the orchestration stays testable. + */ +export interface CatalogGateDeps { + readonly execute: (command: string, args: readonly string[]) => Promise; +} + +const defaultDeps: CatalogGateDeps = { execute }; + +/** + * Reads `pnpm-workspace.yaml` as of the base ref. + * + * A base that genuinely predates the file resolves to `''` (every catalog + * entry then reads as newly added), but any *other* `git show` failure throws + * rather than being folded into that same empty answer. Collapsing the two + * would turn a transient git error into a report that every catalog entry in + * the workspace just changed — the loudest possible false positive. + */ +export const readBaseWorkspace = async ( + base: string, + deps: CatalogGateDeps, +): Promise => { + const resolved = await deps.execute('git', ['rev-parse', '--verify', base]); + if (resolved.exitCode !== 0) { + throw new Error( + `cannot resolve base ref '${base}' — fetch it before running this check`, + ); + } + const target = `${base}:${workspaceFileName}`; + const exists = await deps.execute('git', ['cat-file', '-e', target]); + if (exists.exitCode !== 0) { + return ''; + } + const shown = await deps.execute('git', ['show', target]); + if (shown.exitCode !== 0) { + throw new Error(`git show ${target} failed: ${shown.stderr}`); + } + + return shown.stdout; +}; + +/** + * Options for {@link runChangesetCheck}. + */ +export interface RunChangesetCheckOptions { + readonly base?: string; + readonly cwd?: string; + readonly ignored?: ReadonlySet; +} + +/** + * Reads the workspace and git state, then applies {@link checkCatalogGate}. + */ +export const runChangesetCheck = async ( + options: RunChangesetCheckOptions = {}, + deps: CatalogGateDeps = defaultDeps, +): Promise => { + const discovery = discoverWorkspace( + options.cwd === undefined ? undefined : { cwd: options.cwd }, + ); + const ignored = new Set([ + ...readConfiguredIgnores(discovery.rootDir), + ...(options.ignored ?? []), + ]); + + return checkCatalogGate({ + baseWorkspace: await readBaseWorkspace(options.base ?? defaultBaseRef, deps), + changesetSources: readChangesetSources(discovery.rootDir), + headWorkspace: readHeadWorkspace(discovery.rootDir), + ignored, + packages: discovery.packages, + }); +}; + +/** + * Parsed citty args for the `check` subcommand. + */ +export interface ChangesetCheckCommandArgs { + readonly cwd?: string | undefined; + readonly since?: string | undefined; +} + +/** + * Runs the gate and reports drift through the given logger. Returns the exit + * code so the citty wrapper can set `process.exitCode` and tests can assert + * without mutating process state. + */ +export const changesetCheckCommand = async ( + rawArgs: readonly string[], + args: ChangesetCheckCommandArgs, + logger: Logger, + deps: CatalogGateDeps = defaultDeps, +): Promise => { + const drift = await runChangesetCheck( + { + ...(args.since !== undefined && { base: args.since }), + ...(args.cwd !== undefined && { cwd: args.cwd }), + ignored: parseIgnoreArgs(rawArgs), + }, + deps, + ); + + if (drift.length === 0) { + logger.info('changeset check passed — no uncovered catalog changes'); + + return 0; + } + + for (const message of drift) { + logger.error(message); + } + logger.error( + "add a changeset for the packages above, or 'pnpm changeset --empty' if " + + 'the new range genuinely needs no release', + ); + + return 1; +}; + +const check = defineCommand({ + args: { + cwd: { + alias: 'C', + description: 'Workspace root directory (defaults to current working directory)', + type: 'string', + }, + ignore: { + description: 'Skip a specific package', + type: 'string', + }, + since: { + description: `Base ref to diff the catalog against (defaults to ${defaultBaseRef})`, + type: 'string', + }, + }, + meta: { + description: 'Require a changeset for catalog changes that reach published packages', + name: 'check', + }, + run: async ({ rawArgs, args }) => { + const exitCode = await changesetCheckCommand( + rawArgs, + { cwd: args.cwd, since: args.since }, + createLogger(), + ); + if (exitCode !== 0) { + process.exitCode = exitCode; + } + }, +}); + +/** + * `gtb changeset` — changeset gates that `changeset status` cannot express. + */ +export const changeset = defineCommand({ + meta: { + description: 'Changeset coverage checks', + name: rootNames.changeset, + }, + subCommands: { check }, +}); diff --git a/packages/cli/src/commands/root/index.ts b/packages/cli/src/commands/root/index.ts index 4260c743..02ab66a1 100644 --- a/packages/cli/src/commands/root/index.ts +++ b/packages/cli/src/commands/root/index.ts @@ -1,3 +1,4 @@ +export { changeset, checkCatalogGate } from './changeset.ts'; export { rootNames } from './names.ts'; export { prepare } from './prepare.ts'; export { sync } from './sync.ts'; diff --git a/packages/cli/src/commands/root/names.ts b/packages/cli/src/commands/root/names.ts index 9b179f71..d2119411 100644 --- a/packages/cli/src/commands/root/names.ts +++ b/packages/cli/src/commands/root/names.ts @@ -2,6 +2,7 @@ * CLI names for root-level commands. */ export const rootNames = { + changeset: 'changeset', hk: 'hk', prepare: 'prepare', publish: 'publish', diff --git a/packages/cli/src/lib/catalog-gate.ts b/packages/cli/src/lib/catalog-gate.ts new file mode 100644 index 00000000..16d80c74 --- /dev/null +++ b/packages/cli/src/lib/catalog-gate.ts @@ -0,0 +1,202 @@ +import * as v from 'valibot'; +import { parse } from 'yaml'; +import { StringRecord } from './schemas.ts'; +import { localeComparer } from './sort.ts'; + +/* + * The catalog gate closes a hole `changeset status` cannot see. That check maps + * changed *files* to packages, but a pnpm catalog moves every dependency range + * to the workspace root, which belongs to no package — so a bump edits only + * `pnpm-workspace.yaml` and no package looks changed. pnpm still rewrites + * `catalog:` to the concrete range at pack time, so the published manifest of + * every consumer changes with no version bump behind it. + * + * The gate stays quiet because of how Renovate writes these updates: with npm's + * default `rangeStrategy`, an in-range bump touches only `pnpm-lock.yaml`, and + * the catalog is edited only when the new version falls *outside* the declared + * range. A catalog edit is therefore already a range-boundary crossing — + * exactly the consumer-visible set. Flipping `rangeStrategy` to `bump` would + * make every patch update edit the catalog and turn this gate into noise. + */ + +/** + * Catalog name for pnpm's top-level `catalog:` block. pnpm also accepts it + * spelled explicitly as `catalog:default`. + */ +export const defaultCatalogName = 'default'; + +/** + * One catalog-backed dependency declaration: which catalog it resolves + * against, and the dependency name. + */ +export interface CatalogDependency { + readonly catalog: string; + readonly name: string; +} + +/** + * Catalog name → (dependency name → declared range). + */ +export type CatalogMap = ReadonlyMap>; + +// Extracted so the `catalogs` entry below stays within max-nested-calls. +const NamedCatalogs = v.record(v.string(), StringRecord); + +const CatalogsSchema = v.looseObject({ + catalog: v.optional(v.nullable(StringRecord)), + catalogs: v.optional(v.nullable(NamedCatalogs)), +}); + +const mergeCatalog = ( + into: Map>, + name: string, + entries: Record, +): void => { + const existing = into.get(name) ?? new Map(); + for (const [dependency, range] of Object.entries(entries)) { + existing.set(dependency, range); + } + into.set(name, existing); +}; + +/** + * Reads the `catalog:` and `catalogs:` blocks out of a `pnpm-workspace.yaml` + * source. Both revisions are parsed and compared as maps rather than diffed as + * text, so reordering or reformatting the file reports no change. + */ +export const parseCatalogs = (source: string): CatalogMap => { + const parsed = v.parse(CatalogsSchema, parse(source) ?? {}); + const catalogs = new Map>(); + if (parsed.catalog) { + mergeCatalog(catalogs, defaultCatalogName, parsed.catalog); + } + const named = Object.entries(parsed.catalogs ?? {}); + for (const [name, entries] of named) { + mergeCatalog(catalogs, name, entries); + } + + return catalogs; +}; + +/** + * A catalog entry that gained a range or had it rewritten. `from` is + * `undefined` for a newly added entry. + */ +export interface CatalogChange { + readonly catalog: string; + readonly from: string | undefined; + readonly name: string; + readonly to: string; +} + +const compareChanges = (left: CatalogChange, right: CatalogChange): number => + localeComparer(left.catalog, right.catalog) || + localeComparer(left.name, right.name); + +/** + * Reports every catalog entry added or re-ranged between two revisions. + * + * Removals are deliberately not reported: an entry can only be orphaned by a + * `package.json` edit that drops the `catalog:` specifier, and that edit + * already makes the package look changed to `changeset status`. + */ +export const diffCatalogs = ( + base: CatalogMap, + head: CatalogMap, +): readonly CatalogChange[] => { + const changes: CatalogChange[] = []; + for (const [catalog, entries] of head) { + const baseEntries = base.get(catalog); + for (const [name, to] of entries) { + const from = baseEntries?.get(name); + if (from !== to) { + changes.push({ catalog, from, name, to }); + } + } + } + + return changes.toSorted(compareChanges); +}; + +const frontmatterPattern = /^---\r?\n(?.*?)\r?\n?---/sv; + +/** + * Reads the package names a changeset's YAML frontmatter declares. An empty + * changeset (`---\n---`) declares none, which is how a PR opts out of a + * release — so it must not be mistaken for coverage. + */ +export const parseChangesetPackages = (source: string): readonly string[] => { + const body = frontmatterPattern.exec(source)?.groups?.['body']?.trim() ?? ''; + if (body === '') { + return []; + } + const result = v.safeParse(StringRecord, parse(body)); + + return result.success ? Object.keys(result.output) : []; +}; + +/** + * The subset of a discovered package this gate reads. Structurally satisfied + * by `PackageCapabilities`. + */ +export interface CatalogConsumer { + readonly catalogDependencies: readonly CatalogDependency[]; + readonly isPublished: boolean; + readonly name: string; +} + +/** + * A catalog change that reaches a published package with nothing releasing it. + */ +export interface CatalogFinding { + readonly change: CatalogChange; + readonly packageName: string; +} + +/** + * Inputs to {@link findUncoveredCatalogChanges}. + */ +export interface FindUncoveredCatalogChangesOptions { + readonly changes: readonly CatalogChange[]; + /** + * Package names covered by a changeset in this PR. + */ + readonly covered: ReadonlySet; + readonly ignored: ReadonlySet; + readonly packages: readonly CatalogConsumer[]; +} + +/** + * Pairs each catalog change with every published package that publishes it as + * a runtime dependency and has no changeset releasing it. + */ +export const findUncoveredCatalogChanges = ( + options: FindUncoveredCatalogChangesOptions, +): readonly CatalogFinding[] => + options.changes.flatMap(change => + options.packages + .filter(pkg => pkg.isPublished) + .filter(pkg => !options.ignored.has(pkg.name)) + .filter(pkg => !options.covered.has(pkg.name)) + .filter(pkg => + pkg.catalogDependencies.some( + dep => dep.catalog === change.catalog && dep.name === change.name, + )) + .map(pkg => ({ change, packageName: pkg.name }))); + +/** + * Renders a {@link CatalogFinding} as a single drift line. + */ +export const formatCatalogFinding = ( + { change, packageName }: CatalogFinding, +): string => { + const entry = change.catalog === defaultCatalogName + ? `catalog entry '${change.name}'` + : `catalog '${change.catalog}' entry '${change.name}'`; + const transition = change.from === undefined + ? `was added as ${change.to}` + : `changed ${change.from} → ${change.to}`; + + return `pnpm-workspace.yaml: ${entry} ${transition} — '${packageName}' ` + + 'publishes it as a runtime dependency but no changeset covers that package'; +}; diff --git a/packages/cli/src/lib/discovery.ts b/packages/cli/src/lib/discovery.ts index 3306175a..055cb7a2 100644 --- a/packages/cli/src/lib/discovery.ts +++ b/packages/cli/src/lib/discovery.ts @@ -1,6 +1,7 @@ import { existsSync, readFileSync, readdirSync } from 'node:fs'; import path from 'node:path'; import { generateTaskPrefix } from '../commands/task/names.ts'; +import { type CatalogDependency, defaultCatalogName } from './catalog-gate.ts'; import type { Manifest } from './manifest.ts'; import { hasPackageBlock } from './pkl-project.ts'; import { localeComparer } from './sort.ts'; @@ -19,6 +20,11 @@ export interface PackageCapabilities { * Resolved `include` directories from tsconfig.build.json (published packages only). */ readonly buildIncludes: readonly string[]; + /** + * Catalog-backed runtime dependencies, sorted and deduplicated. See + * {@link collectCatalogDependencies}. + */ + readonly catalogDependencies: readonly CatalogDependency[]; /** * Package directory path. */ @@ -260,6 +266,55 @@ const collectWorkspaceDependencies = (manifest: Manifest): readonly string[] => return [...new Set(names)].toSorted(localeComparer); }; +const catalogPrefix = 'catalog:'; + +/** + * Resolves the catalog a specifier reads from, or `undefined` when the + * specifier isn't catalog-backed. Bare `catalog:` and the explicit + * `catalog:default` both name the default catalog. + */ +const resolveCatalogName = (specifier: string): string | undefined => { + if (!specifier.startsWith(catalogPrefix)) { + return undefined; + } + const named = specifier.slice(catalogPrefix.length).trim(); + + return named === '' ? defaultCatalogName : named; +}; + +/** + * Catalog-backed dependencies this package carries into a consumer's install. + * + * pnpm rewrites a `catalog:` specifier to the catalog's concrete range at pack + * time, so the published manifest changes whenever that range does — which is + * why re-ranging a catalog entry is a consumer-visible change even though no + * file inside the package moved. + * + * The field set and the `bundleDependencies` exclusion mirror + * {@link collectWorkspaceDependencies} for the same reasons: publishing strips + * `devDependencies`, and a bundled dependency ships inside the tarball rather + * than resolving from the registry, so neither range reaches a consumer. + */ +export const collectCatalogDependencies = ( + manifest: Manifest, +): readonly CatalogDependency[] => { + const isBundled = bundledPredicate(manifest); + const refs = runtimeDependencyFields.flatMap(field => + Object.entries(manifest[field] ?? {}) + .filter(([name]) => !isBundled(field, name)) + .flatMap(([name, specifier]) => { + const catalog = resolveCatalogName(specifier); + + return catalog === undefined ? [] : [{ catalog, name }]; + }), + ); + const seen = new Map(refs.map(ref => [`${ref.catalog}${ref.name}`, ref])); + + return seen.values().toArray().toSorted((left, right) => + localeComparer(left.catalog, right.catalog) || + localeComparer(left.name, right.name)); +}; + const collectGenerateScripts = (manifest: Manifest): readonly string[] => Object.keys(manifest.scripts ?? {}) .filter(name => name.startsWith(generateTaskPrefix)) @@ -281,6 +336,7 @@ const buildCapabilities = ( return { buildIncludes: isPublished ? resolveBuildIncludes(dir) : buildInclude, + catalogDependencies: collectCatalogDependencies(manifest), dir, generateScripts, hasBin: hasDir(dir, 'bin'), diff --git a/packages/cli/test/catalog-gate.test.ts b/packages/cli/test/catalog-gate.test.ts new file mode 100644 index 00000000..1cb031f0 --- /dev/null +++ b/packages/cli/test/catalog-gate.test.ts @@ -0,0 +1,326 @@ +import * as build from '@gtbuchanan/test-utils/builders'; +import { describe, it } from 'vitest'; +import { + diffCatalogs, + findUncoveredCatalogChanges, + formatCatalogFinding, + parseCatalogs, + parseChangesetPackages, +} from '#src/lib/catalog-gate.js'; +import { collectCatalogDependencies } from '#src/lib/discovery.js'; + +const workspaceYaml = (body: string): string => `packages:\n - 'packages/*'\n${body}`; + +const catalogOf = (name: string, range: string): string => + workspaceYaml(`catalog:\n ${name}: '${range}'\n`); + +const twoCatalogs = ( + name: string, + defaultRange: string, + legacyRange: string, +): string => + workspaceYaml( + `catalog:\n ${name}: '${defaultRange}'\n` + + `catalogs:\n legacy:\n ${name}: '${legacyRange}'\n`, + ); + +const changeOf = (name: string) => ({ + catalog: 'default', + from: '^1.0.0', + name, + to: '^2.0.0', +}); + +const consumerOf = (packageName: string, dependency: string, isPublished = true) => ({ + catalogDependencies: [{ catalog: 'default', name: dependency }], + isPublished, + name: packageName, +}); + +describe.concurrent(parseCatalogs, () => { + it('reads an entry from the default catalog', ({ expect }) => { + const name = build.packageName(); + const range = build.semverRange(); + + const catalogs = parseCatalogs(catalogOf(name, range)); + + expect(catalogs.get('default')?.get(name)).toBe(range); + }); + + it('keeps a named catalog separate from the default', ({ expect }) => { + const name = build.packageName(); + const defaultRange = build.semverRange(); + const namedRange = build.semverRange(); + + const catalogs = parseCatalogs(twoCatalogs(name, defaultRange, namedRange)); + + expect(catalogs.get('default')?.get(name)).toBe(defaultRange); + expect(catalogs.get('legacy')?.get(name)).toBe(namedRange); + }); + + it('reports no catalogs when the workspace declares none', ({ expect }) => { + expect(parseCatalogs(workspaceYaml('')).size).toBe(0); + }); +}); + +describe.concurrent(diffCatalogs, () => { + it('reports an entry whose range changed, with both ranges', ({ expect }) => { + const name = build.packageName(); + + const changes = diffCatalogs( + parseCatalogs(catalogOf(name, '^1.0.0')), + parseCatalogs(catalogOf(name, '^2.0.0')), + ); + + expect(changes).toStrictEqual([ + { catalog: 'default', from: '^1.0.0', name, to: '^2.0.0' }, + ]); + }); + + it('reports an added entry with no previous range', ({ expect }) => { + const name = build.packageName(); + const to = build.semverRange(); + + const changes = diffCatalogs( + parseCatalogs(workspaceYaml('')), + parseCatalogs(catalogOf(name, to)), + ); + + expect(changes).toStrictEqual([ + { catalog: 'default', from: undefined, name, to }, + ]); + }); + + it('ignores an entry whose range is unchanged', ({ expect }) => { + const source = catalogOf(build.packageName(), build.semverRange()); + + expect(diffCatalogs(parseCatalogs(source), parseCatalogs(source))).toStrictEqual([]); + }); + + /* + * A removed entry can only be orphaned by a package.json edit, which + * `changeset status` already sees — so it is not this gate's business. + */ + it('ignores a removed entry', ({ expect }) => { + const source = catalogOf(build.packageName(), build.semverRange()); + + const changes = diffCatalogs( + parseCatalogs(source), + parseCatalogs(workspaceYaml('')), + ); + + expect(changes).toStrictEqual([]); + }); + + it('treats the same dependency in two catalogs as distinct', ({ expect }) => { + const name = build.packageName(); + + const changes = diffCatalogs( + parseCatalogs(twoCatalogs(name, '^1.0.0', '^1.0.0')), + parseCatalogs(twoCatalogs(name, '^2.0.0', '^1.0.0')), + ); + + expect(changes).toStrictEqual([ + { catalog: 'default', from: '^1.0.0', name, to: '^2.0.0' }, + ]); + }); +}); + +describe.concurrent(collectCatalogDependencies, () => { + it('collects a catalog-backed runtime dependency', ({ expect }) => { + const name = build.packageName(); + + const result = collectCatalogDependencies({ dependencies: { [name]: 'catalog:' } }); + + expect(result).toStrictEqual([{ catalog: 'default', name }]); + }); + + it('resolves an explicitly named catalog specifier', ({ expect }) => { + const name = build.packageName(); + + const result = collectCatalogDependencies({ + dependencies: { [name]: 'catalog:legacy' }, + }); + + expect(result).toStrictEqual([{ catalog: 'legacy', name }]); + }); + + it('collects peerDependencies and optionalDependencies', ({ expect }) => { + const peer = build.packageName(); + const optional = build.packageName(); + + const result = collectCatalogDependencies({ + optionalDependencies: { [optional]: 'catalog:' }, + peerDependencies: { [peer]: 'catalog:' }, + }); + + expect(result).toContainEqual({ catalog: 'default', name: peer }); + expect(result).toContainEqual({ catalog: 'default', name: optional }); + }); + + // Publishing strips devDependencies, so they never reach a consumer. + it('excludes devDependencies', ({ expect }) => { + const name = build.packageName(); + + const result = collectCatalogDependencies({ + devDependencies: { [name]: 'catalog:' }, + }); + + expect(result).toStrictEqual([]); + }); + + it('excludes a dependency the tarball bundles', ({ expect }) => { + const name = build.packageName(); + + const result = collectCatalogDependencies({ + bundleDependencies: [name], + dependencies: { [name]: 'catalog:' }, + }); + + expect(result).toStrictEqual([]); + }); + + it('ignores a specifier that is not catalog-backed', ({ expect }) => { + const name = build.packageName(); + + const result = collectCatalogDependencies({ + dependencies: { [name]: build.semverRange() }, + }); + + expect(result).toStrictEqual([]); + }); +}); + +describe.concurrent(parseChangesetPackages, () => { + it('reads the package names a changeset declares', ({ expect }) => { + const first = build.scopedPackageName(); + const second = build.scopedPackageName(); + + const result = parseChangesetPackages( + `---\n'${first}': patch\n'${second}': minor\n---\n\nSome summary\n`, + ); + + expect(new Set(result)).toStrictEqual(new Set([first, second])); + }); + + it('reports no packages for an empty changeset', ({ expect }) => { + expect(parseChangesetPackages('---\n---\n\nUpdate CI workflow\n')).toStrictEqual([]); + }); + + it('reports no packages when the file has no frontmatter', ({ expect }) => { + expect(parseChangesetPackages('Just prose, no frontmatter\n')).toStrictEqual([]); + }); +}); + +describe.concurrent(findUncoveredCatalogChanges, () => { + it('flags a published consumer with no changeset covering it', ({ expect }) => { + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + const findings = findUncoveredCatalogChanges({ + changes: [changeOf(dependency)], + covered: new Set(), + ignored: new Set(), + packages: [consumerOf(packageName, dependency)], + }); + + expect(findings).toStrictEqual([ + { change: changeOf(dependency), packageName }, + ]); + }); + + it('accepts a changeset that covers the consumer', ({ expect }) => { + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + const findings = findUncoveredCatalogChanges({ + changes: [changeOf(dependency)], + covered: new Set([packageName]), + ignored: new Set(), + packages: [consumerOf(packageName, dependency)], + }); + + expect(findings).toStrictEqual([]); + }); + + it('skips a private consumer', ({ expect }) => { + const dependency = build.packageName(); + + const findings = findUncoveredCatalogChanges({ + changes: [changeOf(dependency)], + covered: new Set(), + ignored: new Set(), + packages: [consumerOf(build.scopedPackageName(), dependency, false)], + }); + + expect(findings).toStrictEqual([]); + }); + + it('skips an explicitly ignored consumer', ({ expect }) => { + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + const findings = findUncoveredCatalogChanges({ + changes: [changeOf(dependency)], + covered: new Set(), + ignored: new Set([packageName]), + packages: [consumerOf(packageName, dependency)], + }); + + expect(findings).toStrictEqual([]); + }); + + it('ignores a catalog change no published package consumes', ({ expect }) => { + const findings = findUncoveredCatalogChanges({ + changes: [changeOf(build.packageName())], + covered: new Set(), + ignored: new Set(), + packages: [consumerOf(build.scopedPackageName(), build.packageName())], + }); + + expect(findings).toStrictEqual([]); + }); + + it('does not match a change in a different catalog', ({ expect }) => { + const dependency = build.packageName(); + const change = { catalog: 'legacy', from: '^1.0.0', name: dependency, to: '^2.0.0' }; + + const findings = findUncoveredCatalogChanges({ + changes: [change], + covered: new Set(), + ignored: new Set(), + packages: [consumerOf(build.scopedPackageName(), dependency)], + }); + + expect(findings).toStrictEqual([]); + }); +}); + +describe.concurrent(formatCatalogFinding, () => { + it('names the entry, both ranges, and the package', ({ expect }) => { + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + const message = formatCatalogFinding({ + change: changeOf(dependency), + packageName, + }); + + expect(message).toContain(dependency); + expect(message).toContain('^1.0.0'); + expect(message).toContain('^2.0.0'); + expect(message).toContain(packageName); + }); + + it('renders an added entry without inventing a previous range', ({ expect }) => { + const dependency = build.packageName(); + + const message = formatCatalogFinding({ + change: { catalog: 'default', from: undefined, name: dependency, to: '^2.0.0' }, + packageName: build.scopedPackageName(), + }); + + expect(message).toContain('added'); + expect(message).not.toContain('undefined'); + }); +}); diff --git a/packages/cli/test/changeset-check.test.ts b/packages/cli/test/changeset-check.test.ts new file mode 100644 index 00000000..9a77b193 --- /dev/null +++ b/packages/cli/test/changeset-check.test.ts @@ -0,0 +1,168 @@ +import * as build from '@gtbuchanan/test-utils/builders'; +import { describe, it } from 'vitest'; +import { + type CatalogGateDeps, + checkCatalogGate, + readBaseWorkspace, +} from '#src/commands/root/changeset.js'; +import type { ExecResult } from '#src/lib/process.js'; + +const ok = (stdout: string): ExecResult => ({ exitCode: 0, stderr: '', stdout }); +const fail = (stderr: string): ExecResult => ({ exitCode: 1, stderr, stdout: '' }); + +const catalogYaml = (entries: string): string => + `packages:\n - 'packages/*'\ncatalog:\n${entries}`; + +const catalogOf = (dependency: string, range: string): string => + catalogYaml(` ${dependency}: '${range}'\n`); + +const publishedConsumer = (packageName: string, dependency: string) => ({ + catalogDependencies: [{ catalog: 'default', name: dependency }], + isPublished: true, + name: packageName, +}); + +/** + * Builds deps whose `git` responses are keyed by subcommand. + */ +const depsFor = ( + responses: Readonly>, +): CatalogGateDeps => ({ + execute: (_command, args) => + Promise.resolve(responses[args[0] ?? ''] ?? fail('unexpected call')), +}); + +describe.concurrent(checkCatalogGate, () => { + it('reports a published consumer of a re-ranged entry', ({ expect }) => { + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + const drift = checkCatalogGate({ + baseWorkspace: catalogOf(dependency, '^1.0.0'), + changesetSources: [], + headWorkspace: catalogOf(dependency, '^2.0.0'), + ignored: new Set(), + packages: [publishedConsumer(packageName, dependency)], + }); + + expect(drift).toHaveLength(1); + expect(drift[0]).toContain(packageName); + expect(drift[0]).toContain(dependency); + }); + + it('reports nothing when the catalog is untouched', ({ expect }) => { + const dependency = build.packageName(); + const source = catalogOf(dependency, '^1.0.0'); + + const drift = checkCatalogGate({ + baseWorkspace: source, + changesetSources: [], + headWorkspace: source, + ignored: new Set(), + packages: [publishedConsumer(build.scopedPackageName(), dependency)], + }); + + expect(drift).toStrictEqual([]); + }); + + it('accepts a changeset naming the affected package', ({ expect }) => { + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + const drift = checkCatalogGate({ + baseWorkspace: catalogOf(dependency, '^1.0.0'), + changesetSources: [`---\n'${packageName}': patch\n---\n\nBump it\n`], + headWorkspace: catalogOf(dependency, '^2.0.0'), + ignored: new Set(), + packages: [publishedConsumer(packageName, dependency)], + }); + + expect(drift).toStrictEqual([]); + }); + + /* + * An empty changeset is the documented way to say "no release", which is + * exactly the claim this gate exists to challenge. + */ + it('rejects an empty changeset as coverage', ({ expect }) => { + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + const drift = checkCatalogGate({ + baseWorkspace: catalogOf(dependency, '^1.0.0'), + changesetSources: ['---\n---\n\nUpdate CI workflow\n'], + headWorkspace: catalogOf(dependency, '^2.0.0'), + ignored: new Set(), + packages: [publishedConsumer(packageName, dependency)], + }); + + expect(drift).toHaveLength(1); + }); + + it('accepts a changeset for one package while flagging another', ({ expect }) => { + const dependency = build.packageName(); + const covered = build.scopedPackageName(); + const uncovered = build.scopedPackageName(); + + const drift = checkCatalogGate({ + baseWorkspace: catalogOf(dependency, '^1.0.0'), + changesetSources: [`---\n'${covered}': patch\n---\n\nBump it\n`], + headWorkspace: catalogOf(dependency, '^2.0.0'), + ignored: new Set(), + packages: [ + publishedConsumer(covered, dependency), + publishedConsumer(uncovered, dependency), + ], + }); + + expect(drift).toHaveLength(1); + expect(drift[0]).toContain(uncovered); + }); +}); + +describe.concurrent(readBaseWorkspace, () => { + it('returns the base revision of the workspace file', async ({ expect }) => { + const source = catalogOf(build.packageName(), '^1.0.0'); + const deps = depsFor({ + 'cat-file': ok(''), + 'rev-parse': ok('abc123'), + 'show': ok(source), + }); + + const result = await readBaseWorkspace('origin/main', deps); + + expect(result).toBe(source); + }); + + it('treats a base predating the file as having no catalogs', async ({ expect }) => { + const deps = depsFor({ + 'cat-file': fail('does not exist'), + 'rev-parse': ok('abc123'), + }); + + const result = await readBaseWorkspace('origin/main', deps); + + expect(result).toBe(''); + }); + + it('throws naming the base ref when it cannot be resolved', async ({ expect }) => { + const base = 'origin/nope'; + const deps = depsFor({ 'rev-parse': fail('unknown revision') }); + + await expect(readBaseWorkspace(base, deps)).rejects.toThrow(base); + }); + + /* + * Folding a git failure into the empty-base answer would report every + * catalog entry in the workspace as newly added. + */ + it('throws rather than reporting an empty catalog when show fails', async ({ expect }) => { + const deps = depsFor({ + 'cat-file': ok(''), + 'rev-parse': ok('abc123'), + 'show': fail('corrupt object'), + }); + + await expect(readBaseWorkspace('origin/main', deps)).rejects.toThrow('corrupt object'); + }); +}); diff --git a/packages/cli/test/coverage-codecov-upload.test.ts b/packages/cli/test/coverage-codecov-upload.test.ts index ab8154e4..74be6c63 100644 --- a/packages/cli/test/coverage-codecov-upload.test.ts +++ b/packages/cli/test/coverage-codecov-upload.test.ts @@ -34,6 +34,7 @@ vi.mock(import('#src/lib/process.js'), async (importOriginal) => { */ const capabilities = (name: string): PackageCapabilities => ({ buildIncludes: [], + catalogDependencies: [], dir: faker.system.directoryPath(), generateScripts: [], hasBin: false, diff --git a/packages/cli/test/turbo-config.helpers.ts b/packages/cli/test/turbo-config.helpers.ts index 8a77935d..02389035 100644 --- a/packages/cli/test/turbo-config.helpers.ts +++ b/packages/cli/test/turbo-config.helpers.ts @@ -1,3 +1,4 @@ +import type { CatalogDependency } from '#src/lib/catalog-gate.js'; import type { PackageCapabilities, WorkspaceDiscovery } from '#src/lib/discovery.js'; import { buildInclude } from '#src/lib/tsconfig-gen.js'; @@ -6,6 +7,7 @@ export const makeCapabilities = ( ): PackageCapabilities => { const merged = { buildIncludes: [...buildInclude] as readonly string[], + catalogDependencies: [] as readonly CatalogDependency[], dir: '/fake/pkg', generateScripts: [] as readonly string[], hasBin: false, From f195c41fb07baa75cc185a04acd3b00ab2f30007 Mon Sep 17 00:00:00 2001 From: Taylor Buchanan Date: Sat, 22 Aug 2026 20:28:47 -0500 Subject: [PATCH 2/5] Cover the catalog gate's filesystem and command layers Codecov flagged the patch at 64% against an 80% target: the pure core was tested but the glue that reads `.changeset`, the changesets config, and the workspace file was not, leaving `changeset.ts` at 45%. Adds tests that drive `runChangesetCheck` and `changesetCheckCommand` against a scaffolded temp monorepo, covering changeset discovery on disk, the config `ignore` list, the `--ignore` flag, and both exit codes. A bare directory with none of the files the gate reads is covered too, since a consumer without a catalog or a `.changeset` directory must no-op rather than throw. Also covers the catalog parser's empty-document and `catalogs.default`-merge branches. Only the citty wrapper is left uncovered, matching the other commands. --- packages/cli/test/catalog-gate.test.ts | 24 +++ packages/cli/test/changeset-check.test.ts | 172 ++++++++++++++++++++++ 2 files changed, 196 insertions(+) diff --git a/packages/cli/test/catalog-gate.test.ts b/packages/cli/test/catalog-gate.test.ts index 1cb031f0..4f8216cb 100644 --- a/packages/cli/test/catalog-gate.test.ts +++ b/packages/cli/test/catalog-gate.test.ts @@ -61,6 +61,30 @@ describe.concurrent(parseCatalogs, () => { it('reports no catalogs when the workspace declares none', ({ expect }) => { expect(parseCatalogs(workspaceYaml('')).size).toBe(0); }); + + it('reports no catalogs for an empty document', ({ expect }) => { + expect(parseCatalogs('').size).toBe(0); + }); + + /* + * pnpm spells the top-level block's name `default`, so a repo may declare + * part of it under `catalogs:` — both halves have to land in one catalog. + */ + it('merges a `catalogs.default` block into the top-level catalog', ({ expect }) => { + const top = build.packageName(); + const nested = build.packageName(); + const nestedRange = build.semverRange(); + + const catalogs = parseCatalogs( + workspaceYaml( + `catalog:\n ${top}: '^1.0.0'\n` + + `catalogs:\n default:\n ${nested}: '${nestedRange}'\n`, + ), + ); + + expect(catalogs.get('default')?.get(top)).toBe('^1.0.0'); + expect(catalogs.get('default')?.get(nested)).toBe(nestedRange); + }); }); describe.concurrent(diffCatalogs, () => { diff --git a/packages/cli/test/changeset-check.test.ts b/packages/cli/test/changeset-check.test.ts index 9a77b193..d8c07c49 100644 --- a/packages/cli/test/changeset-check.test.ts +++ b/packages/cli/test/changeset-check.test.ts @@ -1,11 +1,16 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; import * as build from '@gtbuchanan/test-utils/builders'; import { describe, it } from 'vitest'; import { type CatalogGateDeps, + changesetCheckCommand, checkCatalogGate, readBaseWorkspace, + runChangesetCheck, } from '#src/commands/root/changeset.js'; import type { ExecResult } from '#src/lib/process.js'; +import { captureLogger, createTempDir, writeJson } from './helpers.ts'; const ok = (stdout: string): ExecResult => ({ exitCode: 0, stderr: '', stdout }); const fail = (stderr: string): ExecResult => ({ exitCode: 1, stderr, stdout: '' }); @@ -32,6 +37,57 @@ const depsFor = ( Promise.resolve(responses[args[0] ?? ''] ?? fail('unexpected call')), }); +/** + * Deps that answer `git show` with the given base revision of the workspace. + */ +const depsShowing = (baseWorkspace: string): CatalogGateDeps => + depsFor({ + 'cat-file': ok(''), + 'rev-parse': ok('abc123'), + 'show': ok(baseWorkspace), + }); + +interface CatalogWorkspace { + readonly baseWorkspace: string; + readonly dependency: string; + readonly packageName: string; + readonly root: string; +} + +/** + * Scaffolds a temp monorepo whose one published package declares a + * catalog-backed runtime dependency, with the catalog already re-ranged + * relative to the returned base revision. + */ +const createCatalogWorkspace = (): CatalogWorkspace => { + const root = createTempDir(); + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + writeFileSync(path.join(root, 'pnpm-workspace.yaml'), catalogOf(dependency, '^2.0.0')); + writeJson(root, 'package.json', { name: build.packageName(), private: true }); + + const pkgDir = path.join(root, 'packages', build.packageName()); + mkdirSync(pkgDir, { recursive: true }); + writeJson(pkgDir, 'package.json', { + dependencies: { [dependency]: 'catalog:' }, + name: packageName, + publishConfig: { directory: build.publishDirectory() }, + version: build.semverVersion(), + }); + + const changesetDir = path.join(root, '.changeset'); + mkdirSync(changesetDir, { recursive: true }); + writeJson(changesetDir, 'config.json', { ignore: [] }); + + return { + baseWorkspace: catalogOf(dependency, '^1.0.0'), + dependency, + packageName, + root, + }; +}; + describe.concurrent(checkCatalogGate, () => { it('reports a published consumer of a re-ranged entry', ({ expect }) => { const dependency = build.packageName(); @@ -166,3 +222,119 @@ describe.concurrent(readBaseWorkspace, () => { await expect(readBaseWorkspace('origin/main', deps)).rejects.toThrow('corrupt object'); }); }); + +describe.concurrent(runChangesetCheck, () => { + it('flags an uncovered catalog change in a real workspace', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + + const drift = await runChangesetCheck( + { cwd: workspace.root }, + depsShowing(workspace.baseWorkspace), + ); + + expect(drift).toHaveLength(1); + expect(drift[0]).toContain(workspace.packageName); + }); + + it('reads a pending changeset off disk as coverage', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + writeFileSync( + path.join(workspace.root, '.changeset', 'cover.md'), + `---\n'${workspace.packageName}': patch\n---\n\nBump\n`, + ); + + const drift = await runChangesetCheck( + { cwd: workspace.root }, + depsShowing(workspace.baseWorkspace), + ); + + expect(drift).toStrictEqual([]); + }); + + it('skips a package the changesets config ignores', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + writeJson(path.join(workspace.root, '.changeset'), 'config.json', { + ignore: [workspace.packageName], + }); + + const drift = await runChangesetCheck( + { cwd: workspace.root }, + depsShowing(workspace.baseWorkspace), + ); + + expect(drift).toStrictEqual([]); + }); + + /* + * A repo with no catalog, no `.changeset` directory, and no changesets + * config is a valid consumer of this reusable workflow — it must no-op + * rather than throw on the missing files. + */ + it('no-ops on a bare directory with none of the files it reads', async ({ expect }) => { + const root = createTempDir(); + writeJson(root, 'package.json', { name: build.packageName(), private: true }); + + const drift = await runChangesetCheck({ cwd: root }, depsShowing('')); + + expect(drift).toStrictEqual([]); + }); + + it('skips a package passed through the ignored option', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + + const drift = await runChangesetCheck( + { cwd: workspace.root, ignored: new Set([workspace.packageName]) }, + depsShowing(workspace.baseWorkspace), + ); + + expect(drift).toStrictEqual([]); + }); +}); + +describe.concurrent(changesetCheckCommand, () => { + it('exits zero and reports the pass on a clean workspace', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + const captured = captureLogger(); + + const exitCode = await changesetCheckCommand( + [], + { cwd: workspace.root }, + captured.logger, + // Base identical to HEAD, so nothing changed. + depsShowing(catalogOf(workspace.dependency, '^2.0.0')), + ); + + expect(exitCode).toBe(0); + expect(captured.out()).toContain('no uncovered catalog changes'); + }); + + it('exits non-zero and reports the drift plus a remedy', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + const captured = captureLogger(); + + const exitCode = await changesetCheckCommand( + [], + { cwd: workspace.root }, + captured.logger, + depsShowing(workspace.baseWorkspace), + ); + + expect(exitCode).toBe(1); + expect(captured.err()).toContain(workspace.packageName); + expect(captured.err()).toContain('changeset --empty'); + }); + + it('honors a --ignore flag from raw args', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + const captured = captureLogger(); + + const exitCode = await changesetCheckCommand( + ['--ignore', workspace.packageName], + { cwd: workspace.root }, + captured.logger, + depsShowing(workspace.baseWorkspace), + ); + + expect(exitCode).toBe(0); + }); +}); From 7b4733cefdf4b11cd29aba398023d807f1c8a70a Mon Sep 17 00:00:00 2001 From: Taylor Buchanan Date: Mon, 24 Aug 2026 08:26:13 -0500 Subject: [PATCH 3/5] Scope the catalog gate's git commands to the workspace root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runChangesetCheck` passed `--cwd` to discovery and every file read but not to `readBaseWorkspace`, and `execute` spawns without a `cwd`. So the head workspace came from the target directory while the base revision came from whatever directory the process happened to be in. Pointing `--cwd` at another repository therefore either failed to resolve the base ref or, worse, diffed two unrelated `pnpm-workspace.yaml` files and reported every catalog entry as newly added — the same false-positive class the surrounding comment claims the design prevents. `readBaseWorkspace` now takes the root and prefixes every invocation with `git -C`, leaving the `CatalogGateDeps` contract unchanged. The regression test asserts the prefix on every recorded call; it fails against the previous implementation. Also corrects the claim that both gates run from node_modules — with `gtb-from-source` the catalog gate runs from workspace source, and gtb needs node_modules either way for its own dependencies. Reported by CodeRabbit. --- .github/workflows/changeset-check.yml | 6 ++- AGENTS.md | 10 ++-- packages/cli/src/commands/root/changeset.ts | 22 +++++++-- packages/cli/test/changeset-check.test.ts | 55 ++++++++++++++++++--- 4 files changed, 77 insertions(+), 16 deletions(-) diff --git a/.github/workflows/changeset-check.yml b/.github/workflows/changeset-check.yml index 4a4e94cf..343e97a1 100644 --- a/.github/workflows/changeset-check.yml +++ b/.github/workflows/changeset-check.yml @@ -14,8 +14,10 @@ jobs: - uses: gtbuchanan/tooling/.github/actions/mise-setup@main - # Both gates run out of node_modules: `changeset` for the stock check, - # the gtb bin for the catalog gate. + # The stock check runs the installed `changeset` bin. gtb needs + # node_modules either way — for its own dependencies when + # `gtb-from-source` runs it from workspace source, or for the bin itself + # when it doesn't. - uses: gtbuchanan/tooling/.github/actions/pnpm-tasks@main - name: Verify changeset exists diff --git a/AGENTS.md b/AGENTS.md index 71cf64ad..59aae7ae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -372,10 +372,12 @@ through `package.json` scripts backed by `gtb` leaf commands. for PRs that don't need a version bump. `gtb changeset check` then fails a catalog change that reaches a published package with no changeset releasing it — a gap `status` cannot see, since a catalog bump edits only the - workspace root; see the `gtb-build-pipeline` skill. The job installs so both - run from `node_modules`. The gate runs through `gtb`, so the caller must - depend on `@gtbuchanan/cli`; the `gtb-from-source` input (default `false`) - flips the invocation exactly as in `cd.yml`. + workspace root; see the `gtb-build-pipeline` skill. The job installs: the + stock check runs the installed `changeset` bin, and gtb needs `node_modules` + either way. The gate runs through `gtb`, so the caller must depend on + `@gtbuchanan/cli`; the `gtb-from-source` input (default `false`) selects + workspace-source execution (`pnpm run gtb`) over the installed bin + (`pnpm exec gtb`), exactly as in `cd.yml`. - **`dependency-review.yml`** — Two PR gates on newly-changed deps. `Dependency Review` runs `actions/dependency-review-action` (fails on advisories at `fail-on-severity`, default `moderate`, and on diff --git a/packages/cli/src/commands/root/changeset.ts b/packages/cli/src/commands/root/changeset.ts index cbaafac7..8c73d3c1 100644 --- a/packages/cli/src/commands/root/changeset.ts +++ b/packages/cli/src/commands/root/changeset.ts @@ -126,20 +126,30 @@ const defaultDeps: CatalogGateDeps = { execute }; */ export const readBaseWorkspace = async ( base: string, + cwd: string, deps: CatalogGateDeps, ): Promise => { - const resolved = await deps.execute('git', ['rev-parse', '--verify', base]); + /* + * `-C` is what keeps the base revision and the head worktree in the same + * repository. `execute` spawns without a `cwd`, so without this every git + * call would read the process working directory while the rest of the gate + * reads the discovered root — and a `--cwd` pointed elsewhere would diff two + * unrelated workspaces, reporting every catalog entry as newly added. + */ + const git = (args: readonly string[]): Promise => + deps.execute('git', ['-C', cwd, ...args]); + const resolved = await git(['rev-parse', '--verify', base]); if (resolved.exitCode !== 0) { throw new Error( `cannot resolve base ref '${base}' — fetch it before running this check`, ); } const target = `${base}:${workspaceFileName}`; - const exists = await deps.execute('git', ['cat-file', '-e', target]); + const exists = await git(['cat-file', '-e', target]); if (exists.exitCode !== 0) { return ''; } - const shown = await deps.execute('git', ['show', target]); + const shown = await git(['show', target]); if (shown.exitCode !== 0) { throw new Error(`git show ${target} failed: ${shown.stderr}`); } @@ -172,7 +182,11 @@ export const runChangesetCheck = async ( ]); return checkCatalogGate({ - baseWorkspace: await readBaseWorkspace(options.base ?? defaultBaseRef, deps), + baseWorkspace: await readBaseWorkspace( + options.base ?? defaultBaseRef, + discovery.rootDir, + deps, + ), changesetSources: readChangesetSources(discovery.rootDir), headWorkspace: readHeadWorkspace(discovery.rootDir), ignored, diff --git a/packages/cli/test/changeset-check.test.ts b/packages/cli/test/changeset-check.test.ts index d8c07c49..2e0f7b7a 100644 --- a/packages/cli/test/changeset-check.test.ts +++ b/packages/cli/test/changeset-check.test.ts @@ -28,15 +28,39 @@ const publishedConsumer = (packageName: string, dependency: string) => ({ }); /** - * Builds deps whose `git` responses are keyed by subcommand. + * Builds deps whose `git` responses are keyed by subcommand. Every call is + * expected to lead with `-C `, so the subcommand is the third argument. */ const depsFor = ( responses: Readonly>, ): CatalogGateDeps => ({ execute: (_command, args) => - Promise.resolve(responses[args[0] ?? ''] ?? fail('unexpected call')), + Promise.resolve(responses[args[2] ?? ''] ?? fail('unexpected call')), }); +/** + * Deps that record every git invocation alongside a fixed base revision. + */ +const recordingDeps = ( + baseWorkspace: string, +): CatalogGateDeps & { readonly calls: string[][] } => { + const calls: string[][] = []; + + return { + calls, + execute: (_command, args) => { + calls.push([...args]); + const responses: Record = { + 'cat-file': ok(''), + 'rev-parse': ok('abc123'), + 'show': ok(baseWorkspace), + }; + + return Promise.resolve(responses[args[2] ?? ''] ?? fail('unexpected call')); + }, + }; +}; + /** * Deps that answer `git show` with the given base revision of the workspace. */ @@ -185,7 +209,7 @@ describe.concurrent(readBaseWorkspace, () => { 'show': ok(source), }); - const result = await readBaseWorkspace('origin/main', deps); + const result = await readBaseWorkspace('origin/main', '/repo', deps); expect(result).toBe(source); }); @@ -196,7 +220,7 @@ describe.concurrent(readBaseWorkspace, () => { 'rev-parse': ok('abc123'), }); - const result = await readBaseWorkspace('origin/main', deps); + const result = await readBaseWorkspace('origin/main', '/repo', deps); expect(result).toBe(''); }); @@ -205,7 +229,7 @@ describe.concurrent(readBaseWorkspace, () => { const base = 'origin/nope'; const deps = depsFor({ 'rev-parse': fail('unknown revision') }); - await expect(readBaseWorkspace(base, deps)).rejects.toThrow(base); + await expect(readBaseWorkspace(base, '/repo', deps)).rejects.toThrow(base); }); /* @@ -219,7 +243,7 @@ describe.concurrent(readBaseWorkspace, () => { 'show': fail('corrupt object'), }); - await expect(readBaseWorkspace('origin/main', deps)).rejects.toThrow('corrupt object'); + await expect(readBaseWorkspace('origin/main', '/repo', deps)).rejects.toThrow('corrupt object'); }); }); @@ -236,6 +260,25 @@ describe.concurrent(runChangesetCheck, () => { expect(drift[0]).toContain(workspace.packageName); }); + /* + * Without `-C`, git reads the process working directory while the rest of + * the gate reads the discovered root — so a `--cwd` elsewhere would diff two + * unrelated workspaces and report every catalog entry as newly added. + */ + it('runs every git command against the discovered workspace root', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + const deps = recordingDeps(workspace.baseWorkspace); + + await runChangesetCheck({ cwd: workspace.root }, deps); + + const prefixes = deps.calls.map(args => args.slice(0, 2)); + + expect(prefixes.length).toBeGreaterThan(0); + expect(prefixes).toStrictEqual( + prefixes.map(() => ['-C', workspace.root]), + ); + }); + it('reads a pending changeset off disk as coverage', async ({ expect }) => { const workspace = createCatalogWorkspace(); writeFileSync( From 898ef8f34dd7821f1b62a01cf51733d0572c2148 Mon Sep 17 00:00:00 2001 From: Taylor Buchanan Date: Mon, 24 Aug 2026 19:15:03 -0500 Subject: [PATCH 4/5] Tell a missing base file apart from a git failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git cat-file -e :` exits non-zero both when the path is absent at the base and when the object is corrupt or unreadable. Only the first should answer `''`; the second was silently folded into it, so a broken base reported every catalog entry as newly added. That is the exact false positive the guard on `git show` further down exists to prevent — the check was simply defeated one call earlier. `git ls-tree --full-tree --name-only` separates them: an absent path is exit 0 with empty stdout, any real failure is non-zero and now throws. `--full-tree` keeps the pathspec root-relative, matching how `git show` resolves `:`. Reported by CodeRabbit. --- packages/cli/src/commands/root/changeset.ts | 27 +++++++++++++++------ packages/cli/test/changeset-check.test.ts | 25 +++++++++++++++---- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/commands/root/changeset.ts b/packages/cli/src/commands/root/changeset.ts index 8c73d3c1..ef431483 100644 --- a/packages/cli/src/commands/root/changeset.ts +++ b/packages/cli/src/commands/root/changeset.ts @@ -119,10 +119,10 @@ const defaultDeps: CatalogGateDeps = { execute }; * Reads `pnpm-workspace.yaml` as of the base ref. * * A base that genuinely predates the file resolves to `''` (every catalog - * entry then reads as newly added), but any *other* `git show` failure throws - * rather than being folded into that same empty answer. Collapsing the two - * would turn a transient git error into a report that every catalog entry in - * the workspace just changed — the loudest possible false positive. + * entry then reads as newly added), but any *other* git failure throws rather + * than being folded into that same empty answer. Collapsing the two would turn + * a transient git error into a report that every catalog entry in the + * workspace just changed — the loudest possible false positive. */ export const readBaseWorkspace = async ( base: string, @@ -144,11 +144,24 @@ export const readBaseWorkspace = async ( `cannot resolve base ref '${base}' — fetch it before running this check`, ); } - const target = `${base}:${workspaceFileName}`; - const exists = await git(['cat-file', '-e', target]); - if (exists.exitCode !== 0) { + /* + * `ls-tree` rather than `cat-file -e`, because only it separates the two + * outcomes: an absent path is exit 0 with empty stdout, while a corrupt or + * unreadable object is non-zero. `cat-file -e` collapses both into the same + * non-zero, which would quietly reinstate the false positive below. + * `--full-tree` keeps the pathspec root-relative, matching how `git show` + * resolves `:`. + */ + const listed = await git([ + 'ls-tree', '--full-tree', '--name-only', base, '--', workspaceFileName, + ]); + if (listed.exitCode !== 0) { + throw new Error(`git ls-tree ${base} failed: ${listed.stderr}`); + } + if (listed.stdout === '') { return ''; } + const target = `${base}:${workspaceFileName}`; const shown = await git(['show', target]); if (shown.exitCode !== 0) { throw new Error(`git show ${target} failed: ${shown.stderr}`); diff --git a/packages/cli/test/changeset-check.test.ts b/packages/cli/test/changeset-check.test.ts index 2e0f7b7a..caa26ead 100644 --- a/packages/cli/test/changeset-check.test.ts +++ b/packages/cli/test/changeset-check.test.ts @@ -51,7 +51,7 @@ const recordingDeps = ( execute: (_command, args) => { calls.push([...args]); const responses: Record = { - 'cat-file': ok(''), + 'ls-tree': ok('pnpm-workspace.yaml'), 'rev-parse': ok('abc123'), 'show': ok(baseWorkspace), }; @@ -66,7 +66,7 @@ const recordingDeps = ( */ const depsShowing = (baseWorkspace: string): CatalogGateDeps => depsFor({ - 'cat-file': ok(''), + 'ls-tree': ok('pnpm-workspace.yaml'), 'rev-parse': ok('abc123'), 'show': ok(baseWorkspace), }); @@ -204,7 +204,7 @@ describe.concurrent(readBaseWorkspace, () => { it('returns the base revision of the workspace file', async ({ expect }) => { const source = catalogOf(build.packageName(), '^1.0.0'); const deps = depsFor({ - 'cat-file': ok(''), + 'ls-tree': ok('pnpm-workspace.yaml'), 'rev-parse': ok('abc123'), 'show': ok(source), }); @@ -216,7 +216,7 @@ describe.concurrent(readBaseWorkspace, () => { it('treats a base predating the file as having no catalogs', async ({ expect }) => { const deps = depsFor({ - 'cat-file': fail('does not exist'), + 'ls-tree': ok(''), 'rev-parse': ok('abc123'), }); @@ -225,6 +225,21 @@ describe.concurrent(readBaseWorkspace, () => { expect(result).toBe(''); }); + /* + * The absent-file and unreadable-object cases must not collapse: only the + * first may answer `''`, or a corrupt base silently reports every catalog + * entry as newly added. + */ + it('throws when the historical file lookup itself fails', async ({ expect }) => { + const deps = depsFor({ + 'ls-tree': fail('fatal: not a tree object'), + 'rev-parse': ok('abc123'), + }); + + await expect(readBaseWorkspace('origin/main', '/repo', deps)) + .rejects.toThrow('not a tree object'); + }); + it('throws naming the base ref when it cannot be resolved', async ({ expect }) => { const base = 'origin/nope'; const deps = depsFor({ 'rev-parse': fail('unknown revision') }); @@ -238,7 +253,7 @@ describe.concurrent(readBaseWorkspace, () => { */ it('throws rather than reporting an empty catalog when show fails', async ({ expect }) => { const deps = depsFor({ - 'cat-file': ok(''), + 'ls-tree': ok('pnpm-workspace.yaml'), 'rev-parse': ok('abc123'), 'show': fail('corrupt object'), }); From 3d882762adeb352d10c06c914395d57c47f3134d Mon Sep 17 00:00:00 2001 From: Taylor Buchanan Date: Mon, 24 Aug 2026 23:59:05 -0500 Subject: [PATCH 5/5] Fold changeset status into gtb changeset check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workflow ran two gates as separate steps, each naming the base ref independently — the same ref, hardcoded twice, in the file #425 already tracks for getting base resolution wrong. It also left two verbs for one concern: upstream `status` beside our `check`. `gtb changeset check` now runs `changeset status` first, then the catalog diff, sharing one resolved base. Status runs first because its failure subsumes the catalog gate's: with no changesets present every catalog finding would be uncovered too, so reporting both is noise. The invocation follows `gtb version`, which already wraps `changeset version` — spawned through pnpm with inherited stdio so changesets' own diagnostics reach the user verbatim. `changesetCheckCommand` now reports a hard failure as a non-zero exit rather than an unhandled rejection, since a missing changeset or an unresolvable base is the gate's answer, not a crash. Doing this before release keeps it cheap: the command is unpublished, so its contract can change without breaking a consumer's workflow. Test fixtures move to changeset-check.helpers.ts, which keeps the suite under the max-lines cap now that it covers both gates. --- .changeset/catalog-changeset-gate.md | 13 +- .github/workflows/changeset-check.yml | 20 +-- AGENTS.md | 18 +- .../cli/skills/gtb-build-pipeline/SKILL.md | 11 +- packages/cli/src/commands/root/changeset.ts | 79 ++++++-- packages/cli/test/changeset-check.helpers.ts | 139 +++++++++++++++ packages/cli/test/changeset-check.test.ts | 168 +++++++----------- 7 files changed, 300 insertions(+), 148 deletions(-) create mode 100644 packages/cli/test/changeset-check.helpers.ts diff --git a/.changeset/catalog-changeset-gate.md b/.changeset/catalog-changeset-gate.md index cfb8e603..949c4780 100644 --- a/.changeset/catalog-changeset-gate.md +++ b/.changeset/catalog-changeset-gate.md @@ -17,11 +17,12 @@ dependency field, and fails when no changeset covers them. `devDependencies`, bundled dependencies, private packages, and anything in the changesets config's `ignore` are excluded, and an empty changeset does not count as coverage. -`changeset-check.yml` runs it alongside the existing `changeset status` gate, -behind the same `gtb-from-source` input `cd.yml` uses. Since the job now -installs for the gtb bin, the stock check moved from `pnpm dlx` to -`pnpm exec` — the `pnpm-resolve-pinned` indirection existed only to avoid the -install, and it already required `@changesets/cli` to be a root devDependency -to resolve the version from the lockfile. +The command runs `changeset status` first, so one invocation covers both the +stock "a changeset exists" requirement and the catalog gate against a single +base ref instead of specifying it twice. `changeset-check.yml` is now one step, +behind the same `gtb-from-source` input `cd.yml` uses. The +`pnpm-resolve-pinned` + `pnpm dlx` indirection is gone: it existed only to +avoid an install the gtb bin needs anyway, and it already required +`@changesets/cli` to be a root devDependency to resolve from the lockfile. `PackageCapabilities` gains a `catalogDependencies` field. diff --git a/.github/workflows/changeset-check.yml b/.github/workflows/changeset-check.yml index 343e97a1..bee1c2a1 100644 --- a/.github/workflows/changeset-check.yml +++ b/.github/workflows/changeset-check.yml @@ -14,21 +14,19 @@ jobs: - uses: gtbuchanan/tooling/.github/actions/mise-setup@main - # The stock check runs the installed `changeset` bin. gtb needs - # node_modules either way — for its own dependencies when + # `gtb changeset check` shells out to the installed `changeset` bin, and + # gtb needs node_modules either way — for its own dependencies when # `gtb-from-source` runs it from workspace source, or for the bin itself # when it doesn't. - uses: gtbuchanan/tooling/.github/actions/pnpm-tasks@main - - name: Verify changeset exists - run: pnpm exec changeset status --since=origin/main - - # `changeset status` maps changed *files* to packages, so a pnpm catalog - # bump — which edits only the workspace root — reads as "no package - # changed", even though pnpm rewrites `catalog:` to the concrete range at - # pack time and every consumer's published manifest moves. No-ops in a - # repo with no catalog. - - name: Verify catalog changes are released + # Runs `changeset status` for the stock "a changeset exists" requirement, + # then the catalog gate for what it structurally cannot see: a pnpm + # catalog bump edits only the workspace root, so it reads as "no package + # changed" even though pnpm rewrites `catalog:` to the concrete range at + # pack time and every consumer's published manifest moves. One command so + # both resolve the same base ref. + - name: Verify changesets cover this PR run: >- ${{ inputs.gtb-from-source && 'pnpm run gtb' || 'pnpm exec gtb' }} changeset check --since=origin/main diff --git a/AGENTS.md b/AGENTS.md index 59aae7ae..af16b756 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ mise.toml — Pin dev-tool versions for local + CI; postinstall hoo renovate.json — Repo-local Renovate config (extends the shared preset) workflows/ cd.yml — Reusable: gtb version (changesets + manifest sync) then gtb publish (npm via OIDC + non-npm channels) - changeset-check.yml — Reusable: verify a changeset exists, and that catalog changes reaching published packages are released + changeset-check.yml — Reusable: gtb changeset check (changeset exists + catalog changes reaching published packages are released) ci.yml — Reusable: build + slow + e2e + coverage dependency-review.yml — Reusable: scan dep changes (vulns + licenses) pr.yml — Pipeline (PR): ci + changeset + deps + pre-commit @@ -367,14 +367,14 @@ through `package.json` scripts backed by `gtb` leaf commands. `true` in `release.yml` to run gtb from source (`pnpm run gtb`), whose bin these jobs don't build. This mirrors the `gtbPrefix` repo-shape resolution used for generated scripts and mise tasks. -- **`changeset-check.yml`** — Two PR gates on releasability. - `changeset status` verifies a changeset exists; use `pnpm changeset --empty` - for PRs that don't need a version bump. `gtb changeset check` then fails a - catalog change that reaches a published package with no changeset releasing - it — a gap `status` cannot see, since a catalog bump edits only the - workspace root; see the `gtb-build-pipeline` skill. The job installs: the - stock check runs the installed `changeset` bin, and gtb needs `node_modules` - either way. The gate runs through `gtb`, so the caller must depend on +- **`changeset-check.yml`** — Runs `gtb changeset check`, which gates + releasability twice in one command: `changeset status` for the stock "a + changeset exists" requirement (use `pnpm changeset --empty` for PRs that + don't need a version bump), then a catalog gate for what `status` + structurally cannot see, since a catalog bump edits only the workspace root. + One command so both resolve the same base ref; see the `gtb-build-pipeline` + skill. The job installs — gtb needs `node_modules` either way, and it shells + out to the installed `changeset` bin. The caller must depend on `@gtbuchanan/cli`; the `gtb-from-source` input (default `false`) selects workspace-source execution (`pnpm run gtb`) over the installed bin (`pnpm exec gtb`), exactly as in `cd.yml`. diff --git a/packages/cli/skills/gtb-build-pipeline/SKILL.md b/packages/cli/skills/gtb-build-pipeline/SKILL.md index 7cb25536..efb19b63 100644 --- a/packages/cli/skills/gtb-build-pipeline/SKILL.md +++ b/packages/cli/skills/gtb-build-pipeline/SKILL.md @@ -201,11 +201,16 @@ An e2e suite won't catch this on your behalf. A harness that installs each works Invoked via mise (`mise run hk:base`) so hk and its tools resolve from mise. The mise task resolves `gtb` itself per repo shape — see the `mise.tasks.toml` notes above. -## Catalog changeset gate (`gtb changeset check`) +## Changeset gate (`gtb changeset check`) -`gtb changeset check` fails when a pnpm catalog change reaches a published package with no changeset releasing it. It exists because `changeset status` maps changed **files** to packages, and a catalog moves every dependency range to `pnpm-workspace.yaml` at the workspace root — which belongs to no package. The bump therefore reads as "no package changed", while pnpm rewrites `catalog:` to the concrete range at pack time, so every consumer's published manifest moves with no version bump behind it. +`gtb changeset check` is the PR releasability gate, and runs two checks against one base ref: -- `gtb changeset check [--since ] [--ignore ]` — diffs the `catalog:` / `catalogs:` blocks between the base ref (default `origin/main`) and HEAD, maps each changed entry to the published packages declaring it, and exits non-zero for any that no changeset covers. +1. `changeset status`, delegated to changesets, which fails when a versionable package changed and no changeset exists at all. It runs first because its failure subsumes the second — with no changesets present, every catalog finding would be uncovered too. +1. The **catalog gate**, which fails when a pnpm catalog change reaches a published package with no changeset releasing it. It exists because `changeset status` maps changed **files** to packages, and a catalog moves every dependency range to `pnpm-workspace.yaml` at the workspace root — which belongs to no package. The bump therefore reads as "no package changed", while pnpm rewrites `catalog:` to the concrete range at pack time, so every consumer's published manifest moves with no version bump behind it. + +- `gtb changeset check [--since ] [--ignore ]` — the base ref defaults to `origin/main`. The catalog half diffs the `catalog:` / `catalogs:` blocks between that ref and HEAD, maps each changed entry to the published packages declaring it, and exits non-zero for any that no changeset covers. + +Both halves share one command precisely so they cannot drift onto different base refs. Both revisions are parsed and compared as maps, so reordering or reformatting `pnpm-workspace.yaml` reports nothing. Only runtime fields count (`dependencies`, `peerDependencies`, `optionalDependencies`) — publishing strips `devDependencies`, so a test-only bump never fires — and bundled dependencies are excluded, matching how `workspaceDependencies` is collected. Private packages and anything in the changesets config's `ignore` are skipped. An empty changeset is **not** coverage: it is the documented way to say "no release", which is the claim the gate exists to challenge. Removed catalog entries are skipped, since orphaning one requires a `package.json` edit that `changeset status` already sees. diff --git a/packages/cli/src/commands/root/changeset.ts b/packages/cli/src/commands/root/changeset.ts index ef431483..3736498c 100644 --- a/packages/cli/src/commands/root/changeset.ts +++ b/packages/cli/src/commands/root/changeset.ts @@ -13,7 +13,7 @@ import { import { discoverWorkspace } from '../../lib/discovery.ts'; import { readJsonFile } from '../../lib/file-writer.ts'; import { type Logger, createLogger } from '../../lib/logger.ts'; -import { type ExecResult, execute } from '../../lib/process.ts'; +import { type ExecResult, type RunOptions, execute, run } from '../../lib/process.ts'; import { StringArray } from '../../lib/schemas.ts'; import { rootNames } from './names.ts'; import { parseIgnoreArgs } from './verify.ts'; @@ -107,13 +107,15 @@ const readHeadWorkspace = (rootDir: string): string => { }; /** - * Side-effecting git access, injected so the orchestration stays testable. + * Side-effecting git and changesets access, injected so the orchestration + * stays testable. */ export interface CatalogGateDeps { readonly execute: (command: string, args: readonly string[]) => Promise; + readonly run: (command: string, options?: RunOptions) => Promise; } -const defaultDeps: CatalogGateDeps = { execute }; +const defaultDeps: CatalogGateDeps = { execute, run }; /** * Reads `pnpm-workspace.yaml` as of the base ref. @@ -180,7 +182,40 @@ export interface RunChangesetCheckOptions { } /** - * Reads the workspace and git state, then applies {@link checkCatalogGate}. + * Runs `changeset status`, which fails when a versionable package changed and + * no changeset exists at all. Delegated to changesets rather than + * reimplemented, and run with inherited stdio so its own diagnostics reach the + * user verbatim. + * + * This runs first because its failure subsumes the catalog gate's: with no + * changesets present, every catalog finding would be uncovered too, so + * reporting both is redundant noise. + */ +const runChangesetStatus = async ( + base: string, + cwd: string, + deps: CatalogGateDeps, +): Promise => { + try { + await deps.run('pnpm', { + args: ['exec', 'changeset', 'status', `--since=${base}`], + cwd, + }); + } catch { + /* + * Deliberately not "changeset status failed": `pnpm exec` verifies the + * workspace's dependencies first, so this also fires when that step fails + * and changesets never ran. + */ + throw new Error('changeset status did not pass — see the output above'); + } +}; + +/** + * Reads the workspace and git state, then applies both gates: `changeset + * status` for the stock "a changeset exists" requirement, then + * {@link checkCatalogGate} for the catalog changes it cannot see. Both resolve + * the same base ref, which is why they share one command. */ export const runChangesetCheck = async ( options: RunChangesetCheckOptions = {}, @@ -189,17 +224,15 @@ export const runChangesetCheck = async ( const discovery = discoverWorkspace( options.cwd === undefined ? undefined : { cwd: options.cwd }, ); + const base = options.base ?? defaultBaseRef; + await runChangesetStatus(base, discovery.rootDir, deps); const ignored = new Set([ ...readConfiguredIgnores(discovery.rootDir), ...(options.ignored ?? []), ]); return checkCatalogGate({ - baseWorkspace: await readBaseWorkspace( - options.base ?? defaultBaseRef, - discovery.rootDir, - deps, - ), + baseWorkspace: await readBaseWorkspace(base, discovery.rootDir, deps), changesetSources: readChangesetSources(discovery.rootDir), headWorkspace: readHeadWorkspace(discovery.rootDir), ignored, @@ -226,14 +259,26 @@ export const changesetCheckCommand = async ( logger: Logger, deps: CatalogGateDeps = defaultDeps, ): Promise => { - const drift = await runChangesetCheck( - { - ...(args.since !== undefined && { base: args.since }), - ...(args.cwd !== undefined && { cwd: args.cwd }), - ignored: parseIgnoreArgs(rawArgs), - }, - deps, - ); + let drift: readonly string[]; + try { + drift = await runChangesetCheck( + { + ...(args.since !== undefined && { base: args.since }), + ...(args.cwd !== undefined && { cwd: args.cwd }), + ignored: parseIgnoreArgs(rawArgs), + }, + deps, + ); + } catch (error) { + /* + * A hard failure (missing changeset, unresolvable base, unreadable git + * object) is the gate's answer, not a crash — report it as a non-zero exit + * rather than an unhandled rejection. + */ + logger.error(error instanceof Error ? error.message : String(error)); + + return 1; + } if (drift.length === 0) { logger.info('changeset check passed — no uncovered catalog changes'); diff --git a/packages/cli/test/changeset-check.helpers.ts b/packages/cli/test/changeset-check.helpers.ts new file mode 100644 index 00000000..5d31f1a5 --- /dev/null +++ b/packages/cli/test/changeset-check.helpers.ts @@ -0,0 +1,139 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import * as build from '@gtbuchanan/test-utils/builders'; +import type { CatalogGateDeps } from '#src/commands/root/changeset.js'; +import type { ExecResult, RunOptions } from '#src/lib/process.js'; +import { createTempDir, writeJson } from './helpers.ts'; + +/** + * A successful command result carrying the given stdout. + */ +export const ok = (stdout: string): ExecResult => ({ exitCode: 0, stderr: '', stdout }); + +/** + * A failed command result carrying the given stderr. + */ +export const fail = (stderr: string): ExecResult => ({ exitCode: 1, stderr, stdout: '' }); + +/** + * A `pnpm-workspace.yaml` whose default catalog holds one entry. + */ +export const catalogOf = (dependency: string, range: string): string => + `packages:\n - 'packages/*'\ncatalog:\n ${dependency}: '${range}'\n`; + +/** + * A published package declaring one catalog-backed runtime dependency. + */ +export const publishedConsumer = (packageName: string, dependency: string) => ({ + catalogDependencies: [{ catalog: 'default', name: dependency }], + isPublished: true, + name: packageName, +}); + +/** + * A `changeset status` that reports a changeset exists. + */ +const statusPasses = (): Promise => Promise.resolve(); + +/** + * Builds deps whose `git` responses are keyed by subcommand. Every call is + * expected to lead with `-C `, so the subcommand is the third argument. + */ +export const depsFor = ( + responses: Readonly>, +): CatalogGateDeps => ({ + execute: (_command, args) => + Promise.resolve(responses[args[2] ?? ''] ?? fail('unexpected call')), + run: statusPasses, +}); + +/** + * Deps that answer `git show` with the given base revision of the workspace. + */ +export const depsShowing = (baseWorkspace: string): CatalogGateDeps => + depsFor({ + 'ls-tree': ok('pnpm-workspace.yaml'), + 'rev-parse': ok('abc123'), + 'show': ok(baseWorkspace), + }); + +/** + * One recorded `run` invocation. + */ +export interface SpawnedCommand { + readonly command: string; + readonly options?: RunOptions; +} + +/** + * Deps that record every git invocation and every spawned command alongside a + * fixed base revision. + */ +export const recordingDeps = ( + baseWorkspace: string, +): CatalogGateDeps & { + readonly calls: string[][]; + readonly spawned: SpawnedCommand[]; +} => { + const calls: string[][] = []; + const spawned: SpawnedCommand[] = []; + + return { + calls, + execute: (_command, args) => { + calls.push([...args]); + + return Promise.resolve(depsShowing(baseWorkspace).execute(_command, args)); + }, + run: (command, options) => { + spawned.push({ command, ...(options !== undefined && { options }) }); + + return Promise.resolve(); + }, + spawned, + }; +}; + +/** + * A scaffolded temp monorepo with one catalog-backed published package. + */ +export interface CatalogWorkspace { + readonly baseWorkspace: string; + readonly dependency: string; + readonly packageName: string; + readonly root: string; +} + +/** + * Scaffolds a temp monorepo whose one published package declares a + * catalog-backed runtime dependency, with the catalog already re-ranged + * relative to the returned base revision. + */ +export const createCatalogWorkspace = (): CatalogWorkspace => { + const root = createTempDir(); + const dependency = build.packageName(); + const packageName = build.scopedPackageName(); + + writeFileSync(path.join(root, 'pnpm-workspace.yaml'), catalogOf(dependency, '^2.0.0')); + writeJson(root, 'package.json', { name: build.packageName(), private: true }); + + const pkgDir = path.join(root, 'packages', build.packageName()); + mkdirSync(pkgDir, { recursive: true }); + writeJson(pkgDir, 'package.json', { + dependencies: { [dependency]: 'catalog:' }, + name: packageName, + publishConfig: { directory: build.publishDirectory() }, + version: build.semverVersion(), + }); + + const changesetDir = path.join(root, '.changeset'); + mkdirSync(changesetDir, { recursive: true }); + writeJson(changesetDir, 'config.json', { ignore: [] }); + + return { + baseWorkspace: catalogOf(dependency, '^1.0.0'), + dependency, + packageName, + root, + }; +}; diff --git a/packages/cli/test/changeset-check.test.ts b/packages/cli/test/changeset-check.test.ts index caa26ead..e6d163d9 100644 --- a/packages/cli/test/changeset-check.test.ts +++ b/packages/cli/test/changeset-check.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, writeFileSync } from 'node:fs'; +import { writeFileSync } from 'node:fs'; import path from 'node:path'; import * as build from '@gtbuchanan/test-utils/builders'; import { describe, it } from 'vitest'; @@ -9,109 +9,18 @@ import { readBaseWorkspace, runChangesetCheck, } from '#src/commands/root/changeset.js'; -import type { ExecResult } from '#src/lib/process.js'; +import { + catalogOf, + createCatalogWorkspace, + depsFor, + depsShowing, + fail, + ok, + publishedConsumer, + recordingDeps, +} from './changeset-check.helpers.ts'; import { captureLogger, createTempDir, writeJson } from './helpers.ts'; -const ok = (stdout: string): ExecResult => ({ exitCode: 0, stderr: '', stdout }); -const fail = (stderr: string): ExecResult => ({ exitCode: 1, stderr, stdout: '' }); - -const catalogYaml = (entries: string): string => - `packages:\n - 'packages/*'\ncatalog:\n${entries}`; - -const catalogOf = (dependency: string, range: string): string => - catalogYaml(` ${dependency}: '${range}'\n`); - -const publishedConsumer = (packageName: string, dependency: string) => ({ - catalogDependencies: [{ catalog: 'default', name: dependency }], - isPublished: true, - name: packageName, -}); - -/** - * Builds deps whose `git` responses are keyed by subcommand. Every call is - * expected to lead with `-C `, so the subcommand is the third argument. - */ -const depsFor = ( - responses: Readonly>, -): CatalogGateDeps => ({ - execute: (_command, args) => - Promise.resolve(responses[args[2] ?? ''] ?? fail('unexpected call')), -}); - -/** - * Deps that record every git invocation alongside a fixed base revision. - */ -const recordingDeps = ( - baseWorkspace: string, -): CatalogGateDeps & { readonly calls: string[][] } => { - const calls: string[][] = []; - - return { - calls, - execute: (_command, args) => { - calls.push([...args]); - const responses: Record = { - 'ls-tree': ok('pnpm-workspace.yaml'), - 'rev-parse': ok('abc123'), - 'show': ok(baseWorkspace), - }; - - return Promise.resolve(responses[args[2] ?? ''] ?? fail('unexpected call')); - }, - }; -}; - -/** - * Deps that answer `git show` with the given base revision of the workspace. - */ -const depsShowing = (baseWorkspace: string): CatalogGateDeps => - depsFor({ - 'ls-tree': ok('pnpm-workspace.yaml'), - 'rev-parse': ok('abc123'), - 'show': ok(baseWorkspace), - }); - -interface CatalogWorkspace { - readonly baseWorkspace: string; - readonly dependency: string; - readonly packageName: string; - readonly root: string; -} - -/** - * Scaffolds a temp monorepo whose one published package declares a - * catalog-backed runtime dependency, with the catalog already re-ranged - * relative to the returned base revision. - */ -const createCatalogWorkspace = (): CatalogWorkspace => { - const root = createTempDir(); - const dependency = build.packageName(); - const packageName = build.scopedPackageName(); - - writeFileSync(path.join(root, 'pnpm-workspace.yaml'), catalogOf(dependency, '^2.0.0')); - writeJson(root, 'package.json', { name: build.packageName(), private: true }); - - const pkgDir = path.join(root, 'packages', build.packageName()); - mkdirSync(pkgDir, { recursive: true }); - writeJson(pkgDir, 'package.json', { - dependencies: { [dependency]: 'catalog:' }, - name: packageName, - publishConfig: { directory: build.publishDirectory() }, - version: build.semverVersion(), - }); - - const changesetDir = path.join(root, '.changeset'); - mkdirSync(changesetDir, { recursive: true }); - writeJson(changesetDir, 'config.json', { ignore: [] }); - - return { - baseWorkspace: catalogOf(dependency, '^1.0.0'), - dependency, - packageName, - root, - }; -}; - describe.concurrent(checkCatalogGate, () => { it('reports a published consumer of a re-ranged entry', ({ expect }) => { const dependency = build.packageName(); @@ -294,6 +203,45 @@ describe.concurrent(runChangesetCheck, () => { ); }); + it('runs changeset status against the same base ref', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + const deps = recordingDeps(workspace.baseWorkspace); + const base = 'origin/release'; + + await runChangesetCheck({ base, cwd: workspace.root }, deps); + + expect(deps.spawned).toStrictEqual([ + { + command: 'pnpm', + options: { + args: ['exec', 'changeset', 'status', `--since=${base}`], + cwd: workspace.root, + }, + }, + ]); + }); + + /* + * With no changesets at all, every catalog finding would be uncovered too, + * so the stock gate's failure subsumes this one rather than doubling it. + */ + it('fails without diffing the catalog when changeset status fails', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + const gitCalls: string[][] = []; + const deps: CatalogGateDeps = { + execute: (_command, args) => { + gitCalls.push([...args]); + + return Promise.resolve(ok('')); + }, + run: () => Promise.reject(new Error('exited with code 1')), + }; + + await expect(runChangesetCheck({ cwd: workspace.root }, deps)) + .rejects.toThrow('changeset status'); + expect(gitCalls).toStrictEqual([]); + }); + it('reads a pending changeset off disk as coverage', async ({ expect }) => { const workspace = createCatalogWorkspace(); writeFileSync( @@ -382,6 +330,22 @@ describe.concurrent(changesetCheckCommand, () => { expect(captured.err()).toContain('changeset --empty'); }); + it('reports a hard failure as a non-zero exit, not a rejection', async ({ expect }) => { + const workspace = createCatalogWorkspace(); + const captured = captureLogger(); + const deps: CatalogGateDeps = { + execute: () => Promise.resolve(ok('')), + run: () => Promise.reject(new Error('exited with code 1')), + }; + + const exitCode = await changesetCheckCommand( + [], { cwd: workspace.root }, captured.logger, deps, + ); + + expect(exitCode).toBe(1); + expect(captured.err()).toContain('changeset status'); + }); + it('honors a --ignore flag from raw args', async ({ expect }) => { const workspace = createCatalogWorkspace(); const captured = captureLogger();