Skip to content

ci(root): expand the lockstep group over the packages a release resolves #676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mobeenabdullah merged 1 commit into from
Aug 11, 2026
Merged

ci(root): expand the lockstep group over the packages a release resolves #676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
"@changesets/parse": "^0.4.3",
"@commitlint/cli": "^20.1.0",
"@commitlint/config-conventional": "^20.0.0",
"@manypkg/get-packages": "^1.1.3",
"@testing-library/jest-dom": "^6.9.1",
"@testing-library/react": "^16.3.0",
"@testing-library/user-event": "^14.6.1",
Expand Down
13 changes: 5 additions & 8 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

62 changes: 44 additions & 18 deletions scripts/release/check-changesets.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { fileURLToPath } from "node:url";
import parseChangeset from "@changesets/parse";
import micromatch from "micromatch";

import { getWorkspacePackageNames } from "./lib.mjs";
import { getWorkspacePackages } from "./lib.mjs";

const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), "..", "..");

Expand All @@ -58,11 +58,13 @@ const ALPHA_BUMP = "patch";
* `[["a", "b"]]` and is refused by the release tooling, and an entry that is not
* a non-empty string names nothing.
*
* Checked here rather than by handing the file to `@changesets/config`, whose
* `parse` needs a resolved workspace from `@manypkg/get-packages` — a second
* dependency, for a rule that is three sentences long. That is the opposite trade
* to the YAML reader, and for the opposite reason: this rule cannot drift, and a
* frontmatter grammar can.
* Checked here rather than by handing the file to `@changesets/config`, and NOT
* because that would cost a dependency — the workspace its `parse` needs is
* resolved a few lines above. Because it would answer a different question.
* Changesets accepts any number of disjoint groups and objects only to a package
* appearing in two of them; "exactly one group" is this repository's rule, which
* upstream will never enforce on its behalf. Delegating the check would silently
* drop the part that matters most here.
*/
export function fixedGroupShape(config) {
const groups = config.fixed;
Expand Down Expand Up @@ -140,25 +142,45 @@ export function lockstepPackages(configText, workspaceNames) {
* config packages. Measuring against "what we publish" would report those four as
* errors on every run.
*
* Checked in BOTH directions. A name in the group that no package answers to is
* a rename or a deletion that the config still believes in, and Changesets fails
* a release on an unknown package in `fixed`.
* Checked in BOTH directions, and the second direction has two causes that are
* NOT the same fault. A name no workspace package answers to is a rename or a
* deletion the config still believes in, and Changesets refuses the release
* outright. A name that DOES resolve but sits outside `packages/` is the
* opposite: the release proceeds, quietly widened. It matters because the group
* moves every member to the highest version any of them carries, so one package
* versioned on its own line — an app, a harness — decides the number the whole
* train takes. Reporting both under one sentence would tell an author the release
* will fail when it is instead going to succeed at the wrong version.
*/
export function groupMatchesWorkspace(packages, workspaceNames) {
export function groupMatchesWorkspace(packages, workspace) {
const problems = [];
const absent = workspaceNames.filter(name => !packages.includes(name));
const absent = workspace.underPackagesDir.filter(
name => !packages.includes(name)
);
if (absent.length > 0) {
problems.push(
`.changeset/config.json: the \`fixed\` group is missing ${absent.join(", ")}. ` +
`Every package under packages/ versions with the group; one left out is ` +
`stranded at an older version by the next release.`
);
}
const unknown = packages.filter(name => !workspaceNames.includes(name));
const outside = packages.filter(
name =>
!workspace.underPackagesDir.includes(name) && workspace.all.includes(name)
);
if (outside.length > 0) {
problems.push(
`.changeset/config.json: the \`fixed\` group reaches ${outside.join(", ")}, ` +
`which is in the workspace but not under packages/. The group takes the ` +
`highest version any member carries, so a package versioned on its own ` +
`line decides the version of the entire release train.`
);
}
const unknown = packages.filter(name => !workspace.all.includes(name));
if (unknown.length > 0) {
problems.push(
`.changeset/config.json: the \`fixed\` group names ${unknown.join(", ")}, ` +
`which no package under packages/ answers to. Changesets refuses a release ` +
`which no package in the workspace answers to. Changesets refuses a release ` +
`on an unknown package in \`fixed\`.`
);
}
Expand Down Expand Up @@ -195,7 +217,8 @@ export function declaredReleases(fileText) {
return undefined;
}
const releases = new Map();
for (const release of parsed.releases) releases.set(release.name, release.type);
for (const release of parsed.releases)
releases.set(release.name, release.type);
return releases;
}

Expand Down Expand Up @@ -259,8 +282,11 @@ export function problemsWith(path, fileText, packages) {
* because the PR that adds a package is often exactly that one — and a stale
* group makes every later changeset wrong while each of them passes.
*/
export function checkChangesets(paths, readFile, configText, workspaceNames) {
const packages = lockstepPackages(configText, workspaceNames);
export function checkChangesets(paths, readFile, configText, workspace) {
// Expanded against the WHOLE workspace, which is what Changesets expands
// against. Narrowing it here to the packages the group is supposed to contain
// would make a pattern reaching outside `packages/` look like exact coverage.
const packages = lockstepPackages(configText, workspace.all);
if (packages.length === 0) {
// A config with no fixed group would make every check below vacuous, and a
// guard that passes because it found nothing to check is worse than none.
Expand All @@ -275,7 +301,7 @@ export function checkChangesets(paths, readFile, configText, workspaceNames) {
// reading the release tooling does not share.
if (shape.length > 0) return shape;
return [
...groupMatchesWorkspace(packages, workspaceNames),
...groupMatchesWorkspace(packages, workspace),
...paths.flatMap(path => problemsWith(path, readFile(path), packages)),
];
}
Expand Down Expand Up @@ -349,7 +375,7 @@ async function main(argv) {
paths,
path => readFileSync(resolve(REPO_ROOT, path), "utf8"),
readFileSync(resolve(REPO_ROOT, ".changeset", "config.json"), "utf8"),
getWorkspacePackageNames()
getWorkspacePackages()
);
if (problems.length === 0) {
console.log(
Expand Down
110 changes: 87 additions & 23 deletions scripts/release/check-changesets.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,21 @@ import {
const CONFIG = JSON.stringify({
fixed: [["nextly", "@nextlyhq/ui", "@nextlyhq/builder"]],
});
const WORKSPACE = ["nextly", "@nextlyhq/ui", "@nextlyhq/builder"];
const PACKAGES = lockstepPackages(CONFIG, WORKSPACE);
const MEMBERS = ["nextly", "@nextlyhq/ui", "@nextlyhq/builder"];

/** A workspace whose every package is one the group is supposed to contain. */
const WORKSPACE = { all: MEMBERS, underPackagesDir: MEMBERS };

/**
* The same workspace plus an app, which is the shape the real repository has:
* in the workspace, versioned on its own line, deliberately NOT on the train.
*/
const WITH_AN_APP = {
all: [...MEMBERS, "playground"],
underPackagesDir: MEMBERS,
};

const PACKAGES = lockstepPackages(CONFIG, WORKSPACE.all);

/** A changeset naming the given `package: bump` pairs. */
function changeset(pairs, body = "Something changed.") {
Expand All @@ -34,7 +47,7 @@ describe("reading the group", () => {
// A guard that passes because it found nothing to check is worse than no
// guard: it reports success on every changeset for as long as the config is
// broken.
const problems = checkChangesets(["a.md"], () => COMPLETE, "{}", PACKAGES);
const problems = checkChangesets(["a.md"], () => COMPLETE, "{}", WORKSPACE);
expect(problems).toHaveLength(1);
expect(problems[0]).toContain("no `fixed` group");
});
Expand Down Expand Up @@ -127,17 +140,24 @@ describe("what else a frontmatter can get wrong", () => {
describe("the group against the workspace", () => {
it("accepts a group that matches", () => {
// The control, and the state the repository is in today.
expect(groupMatchesWorkspace(PACKAGES, [...PACKAGES])).toEqual([]);
expect(groupMatchesWorkspace(PACKAGES, WORKSPACE)).toEqual([]);
});

it("accepts a group that leaves an app out of the train", () => {
// The other control, and the one the real config relies on: a workspace
// package outside packages/ is not a gap in the group. A check measuring the
// group against the whole workspace would report `playground` on every run.
expect(groupMatchesWorkspace(PACKAGES, WITH_AN_APP)).toEqual([]);
});

it("catches a package the config was never told about", () => {
// The drift the config cannot see about itself: a PR adds a package under
// packages/ and every changeset in it names the old members, so all of them
// pass while the new package is stranded on the next train.
const problems = groupMatchesWorkspace(PACKAGES, [
...PACKAGES,
"@nextlyhq/newcomer",
]);
const problems = groupMatchesWorkspace(PACKAGES, {
all: [...MEMBERS, "@nextlyhq/newcomer"],
underPackagesDir: [...MEMBERS, "@nextlyhq/newcomer"],
});

expect(problems).toHaveLength(1);
expect(problems[0]).toContain("@nextlyhq/newcomer");
Expand All @@ -149,21 +169,43 @@ describe("the group against the workspace", () => {
// release on an unknown package in `fixed`.
const problems = groupMatchesWorkspace(
[...PACKAGES, "@nextlyhq/departed"],
[...PACKAGES]
WORKSPACE
);

expect(problems).toHaveLength(1);
expect(problems[0]).toContain("@nextlyhq/departed");
expect(problems[0]).toContain("refuses a release");
});

it("separates a group reaching outside packages/ from a name that resolves to nothing", () => {
// Two different outcomes, so they cannot share a sentence. A name nothing
// answers to fails the release; a name answering to an app succeeds it, at
// whatever version that app happens to carry.
const problems = groupMatchesWorkspace(
[...PACKAGES, "playground", "@nextlyhq/departed"],
WITH_AN_APP
);

expect(problems).toHaveLength(2);
const reaches = problems.find(problem => problem.includes("playground"));
expect(reaches).toContain("not under packages/");
expect(reaches).not.toContain("@nextlyhq/departed");

const unknown = problems.find(problem =>
problem.includes("@nextlyhq/departed")
);
expect(unknown).toContain("refuses a release");
expect(unknown).not.toContain("playground");
});

it("is asked even when the pull request touches no changeset", () => {
// The PR that adds a package is often exactly the one with no changeset of
// its own, so an early return on an empty list would skip the check that
// matters most.
const problems = checkChangesets([], () => "", CONFIG, [
...PACKAGES,
"@nextlyhq/newcomer",
]);
const problems = checkChangesets([], () => "", CONFIG, {
all: [...MEMBERS, "@nextlyhq/newcomer"],
underPackagesDir: [...MEMBERS, "@nextlyhq/newcomer"],
});

expect(problems).toHaveLength(1);
expect(problems[0]).toContain("@nextlyhq/newcomer");
Expand Down Expand Up @@ -295,7 +337,7 @@ describe("the shape of the fixed group itself", () => {
[],
() => "",
JSON.stringify({ fixed: ["nextly", "@nextlyhq/ui"] }),
["nextly", "@nextlyhq/ui", "@nextlyhq/builder"]
WORKSPACE
);
expect(problems).toHaveLength(1);
expect(problems[0]).toContain("array of ARRAYS");
Expand All @@ -309,8 +351,8 @@ describe("a group written with globs", () => {
// report every matching package as missing and the pattern as unknown, which
// on a config written this way rejects every pull request.
const globbed = JSON.stringify({ fixed: [["nextly", "@nextlyhq/*"]] });
expect(lockstepPackages(globbed, WORKSPACE).sort()).toEqual(
[...WORKSPACE].sort()
expect(lockstepPackages(globbed, WORKSPACE.all).sort()).toEqual(
[...MEMBERS].sort()
);
});

Expand All @@ -319,22 +361,44 @@ describe("a group written with globs", () => {
// `@changesets/config` does it. Evaluating each pattern alone and unioning
// the results would let `**` put back what `!…` excluded.
const negated = JSON.stringify({ fixed: [["**", "!@nextlyhq/builder"]] });
expect(lockstepPackages(negated, WORKSPACE).sort()).toEqual([
expect(lockstepPackages(negated, WORKSPACE.all).sort()).toEqual([
"@nextlyhq/ui",
"nextly",
]);
});

it("reports a pattern that matches nothing by the name it was written as", () => {
const globbed = JSON.stringify({ fixed: [["@nowhere/*"]] });
expect(lockstepPackages(globbed, WORKSPACE)).toEqual(["@nowhere/*"]);
expect(lockstepPackages(globbed, WORKSPACE.all)).toEqual(["@nowhere/*"]);
});

it("does not report the workspace as missing when a glob covers it", () => {
const globbed = JSON.stringify({ fixed: [["nextly", "@nextlyhq/*"]] });
expect(
checkChangesets([], () => "", globbed, WORKSPACE)
).toEqual([]);
expect(checkChangesets([], () => "", globbed, WORKSPACE)).toEqual([]);
});

it("expands a broad pattern over the packages a release resolves, not one directory", () => {
// The gap this closes. `**` reaches every package Changesets enumerates,
// which includes the ones outside packages/. Expanding it against the
// packages/ listing alone answers with exact coverage, so a changeset naming
// only those packages passes here and the release widens underneath it.
const globbed = JSON.stringify({ fixed: [["**"]] });

expect(lockstepPackages(globbed, WITH_AN_APP.all).sort()).toEqual(
[...WITH_AN_APP.all].sort()
);

const problems = checkChangesets([], () => "", globbed, WITH_AN_APP);
expect(problems).toHaveLength(1);
expect(problems[0]).toContain("playground");
expect(problems[0]).toContain("not under packages/");
});

it("still accepts a broad pattern in a workspace that is only packages/", () => {
// The other direction, so the case above cannot pass by rejecting `**` on
// sight: with nothing outside packages/ to reach, the same group is correct.
const globbed = JSON.stringify({ fixed: [["**"]] });
expect(checkChangesets([], () => "", globbed, WORKSPACE)).toEqual([]);
});
});

Expand Down Expand Up @@ -403,7 +467,7 @@ describe("what the check is pointed at", () => {
["good.md", "bad.md"],
path => files[path],
CONFIG,
PACKAGES
WORKSPACE
);

expect(problems).toHaveLength(1);
Expand All @@ -413,6 +477,6 @@ describe("what the check is pointed at", () => {
it("passes when it is given nothing", () => {
// The ordinary PR touches no changeset — a test-only or docs-only one gets
// none at all — and that must not be a failure.
expect(checkChangesets([], () => "", CONFIG, PACKAGES)).toEqual([]);
expect(checkChangesets([], () => "", CONFIG, WORKSPACE)).toEqual([]);
});
});
Loading
Loading