✨ Add cascade version bump tooling (pnpm versions / pnpm versions:sync)#185
✨ Add cascade version bump tooling (pnpm versions / pnpm versions:sync)#185
Conversation
📝 WalkthroughWalkthroughAdds cascade version-bump tooling for PNPM workspaces: dependency-graph utilities, a check/sync script to detect/apply transitive version bumps for published dependents, CI step to run the check, npm scripts, and policy documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as pnpm versions
participant CheckVer as check-versions.ts
participant DepGraph as dep-graph.ts
participant NPM as npm Registry
participant FileSystem as filesystem
User->>CLI: Execute pnpm versions
CLI->>CheckVer: Invoke check mode
CheckVer->>DepGraph: buildDepGraph()
DepGraph->>FileSystem: Read pnpm-workspace.yaml
DepGraph->>FileSystem: Read package.json files
DepGraph-->>CheckVer: Return graph with packages & dependents
loop For each package
CheckVer->>NPM: npm view to check published version
NPM-->>CheckVer: Version or not-found
end
alt Package not published
CheckVer->>DepGraph: getTransitiveDependents()
DepGraph-->>CheckVer: Set of dependent packages
CheckVer->>CheckVer: Identify required bumps
CheckVer->>User: Report missing cascade bumps
end
sequenceDiagram
actor User
participant CLI as pnpm versions:sync
participant CheckVer as check-versions.ts
participant DepGraph as dep-graph.ts
participant NPM as npm Registry
participant FileSystem as filesystem
User->>CLI: Execute pnpm versions:sync
CLI->>CheckVer: Invoke sync mode
CheckVer->>DepGraph: buildDepGraph()
DepGraph->>FileSystem: Read workspace and packages
DepGraph-->>CheckVer: Return dependency graph
loop For each package
CheckVer->>NPM: Check if version published
end
loop For unpublished packages
CheckVer->>DepGraph: getTransitiveDependents()
DepGraph-->>CheckVer: Affected dependents
CheckVer->>CheckVer: Compute patch bumps
end
loop Update each dependent
CheckVer->>FileSystem: Read package.json
CheckVer->>CheckVer: Increment version
CheckVer->>FileSystem: Write updated package.json
end
CheckVer->>User: Print summary of bumps applied
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.internal/check-versions.ts:
- Around line 101-115: The current grouping uses pkg.publishedWorkspaceDeps
filtered only by pendingRelease, which loses the transitive cause for packages
in requiredBumps; change the trigger detection in the loop over requiredBumps to
walk dependency edges (starting from each dep in pkg.publishedWorkspaceDeps)
back to the nearest package that is in pendingRelease (or is an original root in
requiredBumps) and record that root trigger so the printed triggerStr shows the
actual triggering package and version; update the logic around requiredBumps /
pkg.publishedWorkspaceDeps / pendingRelease (the block building triggers and
triggerStr) to perform this traversal and map each dependency to its ultimate
triggering package before formatting the output.
- Around line 28-39: The loop over graph.packages currently treats any non-zero
exit from the `npm view ${pkg.name}@${pkg.version}` call (npmCheck/result) as
"version not published" and adds pkg.name to pendingRelease; change this to
distinguish three cases: (1) run `npm view ${pkg.name}@${pkg.version}` and if it
returns exitCode 0 with stdout non-empty, do nothing; (2) if version-check
exitCode !== 0, run a plain `npm view ${pkg.name}` to see if the package
exists—if that exists but version missing, add to pendingRelease; (3) if the
package-check itself failed with a transient/network/auth error (non-zero
exitCode or other error), surface a hard failure (throw or exit) instead of
adding to pendingRelease so `versions:sync` cannot rewrite unrelated packages;
mirror the existence/version split logic used in publish-matrix.ts and use the
same helper/command patterns around npmCheck/result and pendingRelease to locate
and update the behavior.
In @.internal/lib/dep-graph.ts:
- Around line 43-56: This code duplicates a pnpm-workspace.yaml parser and omits
the glob guard; instead reuse the existing parser from .internal/sync-tsrefs.ts:
export the parser there (e.g., parsePnpmWorkspace or parseWorkspaceYaml) and
import it into .internal/lib/dep-graph.ts, replacing the manual loop that builds
workspaces from workspaceYaml and the local workspaces array; ensure the shared
parser preserves the glob-character guard and returns the same string[] shape so
dep-graph.ts can use it without additional logic.
- Around line 145-163: The predicate in collectWorkspaceDeps is too broad
(range.startsWith("workspace:")); narrow it to only match the documented
workspace cascade variants (e.g., workspace:*, workspace:^, workspace:~) by
replacing that check with a strict pattern test on range (for example using a
regex like /^workspace:(\*|~|\^)/) while keeping the existing guards
(depsObject, typeof range) and leaving packageNames and result handling intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43868006-1324-4f27-bb45-4e894a9b0d4b
📒 Files selected for processing (5)
.github/workflows/test.yaml.internal/check-versions.ts.internal/lib/dep-graph.ts.policies/version-bump.mdpackage.json
| for (const pkg of graph.packages.values()) { | ||
| const npmCheck = yield* x( | ||
| "npm", | ||
| ["view", `${pkg.name}@${pkg.version}`, "version"], | ||
| { throwOnError: false }, | ||
| ); | ||
| const result = yield* npmCheck; | ||
|
|
||
| if (result.exitCode !== 0 || result.stdout.trim() === "") { | ||
| pendingRelease.add(pkg.name); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import json
import pathlib
import subprocess
root = pathlib.Path(".")
entries = []
workspace_file = root / "pnpm-workspace.yaml"
for line in workspace_file.read_text().splitlines():
trimmed = line.strip()
if trimmed.startswith("-"):
value = trimmed[1:].strip().strip('"').strip("'")
if value:
entries.append(value)
for workspace in entries:
pkgfile = root / workspace / "package.json"
if not pkgfile.exists():
continue
data = json.loads(pkgfile.read_text())
if data.get("private"):
continue
name = data["name"]
version = data["version"]
pkg = subprocess.run(
["npm", "view", name, "version"],
text=True,
capture_output=True,
)
ver = subprocess.run(
["npm", "view", f"{name}@{version}", "version"],
text=True,
capture_output=True,
)
if pkg.returncode != 0 or ver.returncode != 0:
print(f"{name}@{version}")
print(f" package rc={pkg.returncode} latest={pkg.stdout.strip()!r}")
if pkg.stderr.strip():
print(f" package stderr={pkg.stderr.strip().splitlines()[-1]!r}")
print(f" version rc={ver.returncode} published={ver.stdout.strip()!r}")
if ver.stderr.strip():
print(f" version stderr={ver.stderr.strip().splitlines()[-1]!r}")
PYRepository: thefrontside/effectionx
Length of output: 49
🏁 Script executed:
cat -n .internal/check-versions.tsRepository: thefrontside/effectionx
Length of output: 4469
🏁 Script executed:
head -60 .internal/publish-matrix.tsRepository: thefrontside/effectionx
Length of output: 1719
🏁 Script executed:
rg -n "versions:sync|check-versions" --type tsRepository: thefrontside/effectionx
Length of output: 49
Don't treat every failed npm view as "version not published" — distinguish package existence from transient failure.
The code conflates three distinct cases: version genuinely missing, package never published, and registry/auth/network failure. The last should hard-fail rather than cascade, but this design also loses the first-publish distinction already tracked in .internal/publish-matrix.ts:20-50. In versions:sync mode, a transient npm failure triggers version rewrites on unrelated packages (lines 66-88). Additionally, the cascade failure explanation (lines 101-110) can report empty trigger strings when conditions align.
Unlike publish-matrix.ts, which runs separate npm view ${name} and npm view ${name}@${version} checks to distinguish package existence, check-versions.ts runs only the version check. Any non-zero exit code becomes a false positive pending release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.internal/check-versions.ts around lines 28 - 39, The loop over
graph.packages currently treats any non-zero exit from the `npm view
${pkg.name}@${pkg.version}` call (npmCheck/result) as "version not published"
and adds pkg.name to pendingRelease; change this to distinguish three cases: (1)
run `npm view ${pkg.name}@${pkg.version}` and if it returns exitCode 0 with
stdout non-empty, do nothing; (2) if version-check exitCode !== 0, run a plain
`npm view ${pkg.name}` to see if the package exists—if that exists but version
missing, add to pendingRelease; (3) if the package-check itself failed with a
transient/network/auth error (non-zero exitCode or other error), surface a hard
failure (throw or exit) instead of adding to pendingRelease so `versions:sync`
cannot rewrite unrelated packages; mirror the existence/version split logic used
in publish-matrix.ts and use the same helper/command patterns around
npmCheck/result and pendingRelease to locate and update the behavior.
| // Group by the dependency that triggered the cascade. | ||
| for (const name of requiredBumps) { | ||
| const pkg = graph.packages.get(name)!; | ||
| // Find which of its deps triggered this. | ||
| const triggers = pkg.publishedWorkspaceDeps.filter((dep) => | ||
| pendingRelease.has(dep), | ||
| ); | ||
| const triggerStr = triggers | ||
| .map((t) => { | ||
| const tp = graph.packages.get(t)!; | ||
| return `${t}@${tp.version}`; | ||
| }) | ||
| .join(", "); | ||
|
|
||
| console.error(` ${name} (${pkg.version}) — depends on ${triggerStr}`); |
There was a problem hiding this comment.
Preserve the trigger chain for transitive failures.
For multi-hop cascades, triggers only checks pendingRelease, so packages reached via another required bump can print depends on with no cause. That makes the check-mode output hard to act on for the exact transitive cases this tool is supposed to catch.
🛠️ Minimal fix
- const triggers = pkg.publishedWorkspaceDeps.filter((dep) =>
- pendingRelease.has(dep),
- );
+ const triggers = pkg.publishedWorkspaceDeps.filter(
+ (dep) => pendingRelease.has(dep) || requiredBumps.has(dep),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Group by the dependency that triggered the cascade. | |
| for (const name of requiredBumps) { | |
| const pkg = graph.packages.get(name)!; | |
| // Find which of its deps triggered this. | |
| const triggers = pkg.publishedWorkspaceDeps.filter((dep) => | |
| pendingRelease.has(dep), | |
| ); | |
| const triggerStr = triggers | |
| .map((t) => { | |
| const tp = graph.packages.get(t)!; | |
| return `${t}@${tp.version}`; | |
| }) | |
| .join(", "); | |
| console.error(` ${name} (${pkg.version}) — depends on ${triggerStr}`); | |
| // Group by the dependency that triggered the cascade. | |
| for (const name of requiredBumps) { | |
| const pkg = graph.packages.get(name)!; | |
| // Find which of its deps triggered this. | |
| const triggers = pkg.publishedWorkspaceDeps.filter( | |
| (dep) => pendingRelease.has(dep) || requiredBumps.has(dep), | |
| ); | |
| const triggerStr = triggers | |
| .map((t) => { | |
| const tp = graph.packages.get(t)!; | |
| return `${t}@${tp.version}`; | |
| }) | |
| .join(", "); | |
| console.error(` ${name} (${pkg.version}) — depends on ${triggerStr}`); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.internal/check-versions.ts around lines 101 - 115, The current grouping
uses pkg.publishedWorkspaceDeps filtered only by pendingRelease, which loses the
transitive cause for packages in requiredBumps; change the trigger detection in
the loop over requiredBumps to walk dependency edges (starting from each dep in
pkg.publishedWorkspaceDeps) back to the nearest package that is in
pendingRelease (or is an original root in requiredBumps) and record that root
trigger so the printed triggerStr shows the actual triggering package and
version; update the logic around requiredBumps / pkg.publishedWorkspaceDeps /
pendingRelease (the block building triggers and triggerStr) to perform this
traversal and map each dependency to its ultimate triggering package before
formatting the output.
.internal/lib/dep-graph.ts
Outdated
| const workspaceYaml = yield* call(() => | ||
| fsp.readFile(resolve(rootDir, "pnpm-workspace.yaml"), "utf-8"), | ||
| ); | ||
|
|
||
| const workspaces: string[] = []; | ||
| for (const line of workspaceYaml.split("\n")) { | ||
| const trimmed = line.trim(); | ||
| if (trimmed.startsWith("-")) { | ||
| const value = trimmed.replace(/^-\s*/, "").replace(/^["']|["']$/g, ""); | ||
| if (value) { | ||
| workspaces.push(value); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Share the workspace parser with sync-tsrefs instead of copying it.
This is now a second hand-rolled pnpm-workspace.yaml parser, but it omits the glob-character guard already present in .internal/sync-tsrefs.ts:56-96. The two internal tools can now disagree on valid workspace entries and fail with different errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.internal/lib/dep-graph.ts around lines 43 - 56, This code duplicates a
pnpm-workspace.yaml parser and omits the glob guard; instead reuse the existing
parser from .internal/sync-tsrefs.ts: export the parser there (e.g.,
parsePnpmWorkspace or parseWorkspaceYaml) and import it into
.internal/lib/dep-graph.ts, replacing the manual loop that builds workspaces
from workspaceYaml and the local workspaces array; ensure the shared parser
preserves the glob-character guard and returns the same string[] shape so
dep-graph.ts can use it without additional logic.
| /** Extract workspace package names that use `workspace:*` protocol. */ | ||
| function collectWorkspaceDeps( | ||
| depsObject: Record<string, string> | undefined, | ||
| packageNames: Set<string>, | ||
| ): string[] { | ||
| if (!depsObject || typeof depsObject !== "object") { | ||
| return []; | ||
| } | ||
|
|
||
| const result: string[] = []; | ||
| for (const [name, range] of Object.entries(depsObject)) { | ||
| if ( | ||
| packageNames.has(name) && | ||
| typeof range === "string" && | ||
| range.startsWith("workspace:") | ||
| ) { | ||
| result.push(name); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import json
import pathlib
root = pathlib.Path(".")
entries = []
workspace_file = root / "pnpm-workspace.yaml"
for line in workspace_file.read_text().splitlines():
trimmed = line.strip()
if trimmed.startswith("-"):
value = trimmed[1:].strip().strip('"').strip("'")
if value:
entries.append(value)
for workspace in entries:
pkgfile = root / workspace / "package.json"
if not pkgfile.exists():
continue
data = json.loads(pkgfile.read_text())
for section in ("dependencies", "peerDependencies"):
deps = data.get(section) or {}
if not isinstance(deps, dict):
continue
for name, rng in deps.items():
if isinstance(rng, str) and rng.startswith("workspace:") and rng != "workspace:*":
print(f"{pkgfile}: {section}.{name} = {rng}")
PYRepository: thefrontside/effectionx
Length of output: 49
Code doesn't match documented policy scope for workspace dependency cascade.
The startsWith("workspace:") predicate is broader than the documented workspace:* cascade rule (which covers workspace:^, workspace:~, and other variants). While the repository currently has no non-workspace:* workspace dependencies that would trigger unnecessary bumps, the code should be narrowed to align with the policy:
- range.startsWith("workspace:")
+ range === "workspace:*"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.internal/lib/dep-graph.ts around lines 145 - 163, The predicate in
collectWorkspaceDeps is too broad (range.startsWith("workspace:")); narrow it to
only match the documented workspace cascade variants (e.g., workspace:*,
workspace:^, workspace:~) by replacing that check with a strict pattern test on
range (for example using a regex like /^workspace:(\*|~|\^)/) while keeping the
existing guards (depsObject, typeof range) and leaving packageNames and result
handling intact.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.internal/check-versions.ts:
- Around line 78-85: The current code reparses and reserializes package.json
which can reorder fields and change formatting; instead update only the version
field in the original raw text to preserve ordering/formatting: locate the
"version" property in the raw string (e.g., using a regex that matches the
"version" key and its value while preserving surrounding whitespace/commas),
replace just the value with newVersion, and then pass the modified raw string to
writeTextFile; fall back to the existing JSON.parse/JSON.stringify approach
(using packageJsonPath, readTextFile, writeTextFile, newVersion) only if the
targeted text-replacement fails.
In @.internal/lib/dep-graph.ts:
- Around line 63-64: The code currently calls
readTextFile(resolve(workspacePath, "package.json")) twice (variables raw/json)
during the workspace scan and again for non-private packages; change the flow so
the initial parse (the json object produced after JSON.parse(raw) in the first
pass) is saved and reused instead of re-reading the file—e.g., store the parsed
object in a local variable scoped outside the condition or attach it to the
workspace record, then reference that parsed JSON when checking package privacy
and other fields (avoid a second readTextFile call around lines where
package.json is read again).
- Around line 66-68: The code pushes package entries assuming json.name and
json.version are valid strings (see allPackages push in dep-graph.ts), which can
break on malformed package.json; add validation before pushing: validate that
json.name and json.version exist and are non-empty strings (or use the existing
zod dependency to define a schema and parse json), and if validation fails skip
the package or throw a clear error (include the package path/context in the
message) so downstream consumers of allPackages only receive well-formed {name,
version} entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9176c96e-4465-4c78-9b9f-56ee41bd43a2
📒 Files selected for processing (2)
.internal/check-versions.ts.internal/lib/dep-graph.ts
| const packageJsonPath = resolve(pkg.workspacePath, "package.json"); | ||
| const raw = yield* readTextFile(packageJsonPath); | ||
| const json = JSON.parse(raw) as Record<string, unknown>; | ||
| json.version = newVersion; | ||
| yield* writeTextFile( | ||
| packageJsonPath, | ||
| `${JSON.stringify(json, null, 2)}\n`, | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
JSON serialization may alter field ordering and formatting.
JSON.stringify with 2-space indent is standard, but it won't preserve the original field order or any custom formatting (like trailing commas if present). This is generally acceptable for package.json, but be aware that diffs may show reordering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.internal/check-versions.ts around lines 78 - 85, The current code reparses
and reserializes package.json which can reorder fields and change formatting;
instead update only the version field in the original raw text to preserve
ordering/formatting: locate the "version" property in the raw string (e.g.,
using a regex that matches the "version" key and its value while preserving
surrounding whitespace/commas), replace just the value with newVersion, and then
pass the modified raw string to writeTextFile; fall back to the existing
JSON.parse/JSON.stringify approach (using packageJsonPath, readTextFile,
writeTextFile, newVersion) only if the targeted text-replacement fails.
| const raw = yield* readTextFile(resolve(workspacePath, "package.json")); | ||
| const json = JSON.parse(raw) as Record<string, unknown>; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid reading package.json twice for non-private packages.
The file is read once at line 63 during the initial workspace scan, then read again at line 90 for non-private packages. Store the parsed JSON in the first pass to avoid redundant I/O.
♻️ Suggested approach
// Read all package.json files and build WorkspacePackage entries.
const allPackages: WorkspacePackage[] = [];
+ const packageJsonCache = new Map<string, Record<string, unknown>>();
for (const workspace of workspaces) {
const workspacePath = resolve(rootDir, workspace);
const raw = yield* readTextFile(resolve(workspacePath, "package.json"));
const json = JSON.parse(raw) as Record<string, unknown>;
+ packageJsonCache.set(workspace, json);
allPackages.push({
name: json.name as string,
// ...
});
}
// Later, when resolving deps:
for (const pkg of packages.values()) {
- const raw = yield* readTextFile(resolve(pkg.workspacePath, "package.json"));
- const json = JSON.parse(raw) as Record<string, unknown>;
+ const json = packageJsonCache.get(pkg.workspace)!;
const deps = collectWorkspaceDeps(
json.dependencies as Record<string, string> | undefined,
packageNames,
);Also applies to: 89-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.internal/lib/dep-graph.ts around lines 63 - 64, The code currently calls
readTextFile(resolve(workspacePath, "package.json")) twice (variables raw/json)
during the workspace scan and again for non-private packages; change the flow so
the initial parse (the json object produced after JSON.parse(raw) in the first
pass) is saved and reused instead of re-reading the file—e.g., store the parsed
object in a local variable scoped outside the condition or attach it to the
workspace record, then reference that parsed JSON when checking package privacy
and other fields (avoid a second readTextFile call around lines where
package.json is read again).
| allPackages.push({ | ||
| name: json.name as string, | ||
| version: json.version as string, |
There was a problem hiding this comment.
Missing validation for required name and version fields.
The code assumes json.name and json.version exist and are strings. A malformed package.json (or workspace pointing to a non-package directory) would cause subtle failures downstream. Add validation or use a schema library like zod (already a dependency per manifest).
🛡️ Suggested validation
+ if (typeof json.name !== "string" || !json.name) {
+ throw new Error(`Missing or invalid "name" in ${workspacePath}/package.json`);
+ }
+ if (typeof json.version !== "string" || !json.version) {
+ throw new Error(`Missing or invalid "version" in ${workspacePath}/package.json`);
+ }
+
allPackages.push({
name: json.name as string,
version: json.version as string,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.internal/lib/dep-graph.ts around lines 66 - 68, The code pushes package
entries assuming json.name and json.version are valid strings (see allPackages
push in dep-graph.ts), which can break on malformed package.json; add validation
before pushing: validate that json.name and json.version exist and are non-empty
strings (or use the existing zod dependency to define a schema and parse json),
and if validation fails skip the package or throw a clear error (include the
package path/context in the message) so downstream consumers of allPackages only
receive well-formed {name, version} entries.
|
What about using convector here? |
Motivation
When a package like
@effectionx/test-adaptergets a version bump, its published dependents (like@effectionx/bddwhich has"@effectionx/test-adapter": "workspace:*"independencies) must also be bumped so they get republished with the updated dependency range. This was previously a manual process and easy to forget — we just hit this exact problem with the bdd/test-adapter relationship (#183).Approach
Two new commands backed by Effection-based scripts in
.internal/:pnpm versionspnpm versions:syncHow it works:
workspace:*entries independenciesandpeerDependencies(ignoresdevDependenciessince they're stripped at publish time)Files added/changed:
.internal/lib/dep-graph.ts— shared dependency graph builder.internal/check-versions.ts— main script with check/sync modespackage.json— addedversionsandversions:syncscripts.github/workflows/test.yaml— added cascade check to CI lint job.policies/version-bump.md— documented cascade rule and toolingSummary by CodeRabbit
New Features
pnpm versionsandpnpm versions:synccommands to detect and apply cascade bumpsDocumentation