fix(cli): validate engines.{aube,pnpm} + workspace engines.node + dist-tag preference by jdx · Pull Request #458 · endevco/aube

jdx · 2026-05-01T15:53:13Z

Summary

Extends crates/aube/src/engines.rs to cover engine constraints aube previously ignored, then ports the three pnpm misc.ts tests this unblocks.

Engines coverage changes

Constraint	Before	After
`engines.node` on root manifest	checked	unchanged
`engines.node` on workspace project manifests	skipped	checked (warn / engine-strict fail)
`engines.node` on transitive deps	checked	unchanged
`engines.aube` on root + workspace projects	skipped	checked against `env!(\"CARGO_PKG_VERSION\")`
`engines.pnpm` on root + workspace projects	skipped	checked against `env!(\"CARGO_PKG_VERSION\")`
`engines.{aube,pnpm}` on transitive deps	skipped	stays skipped (wild packages routinely pin authors' toolchains; would drown users in warnings)

aube positions itself as a pnpm-compatible drop-in, so a package gating on engines.pnpm is honored as if aube were that pnpm.

The warning printer now labels each mismatch with the engine field that triggered it (wanted node >=20, wanted aube >=99999, wanted pnpm >=8). The strict-mode error is now engine-agnostic — existing engines.bats assertions on the engine-strict substring keep working.

Tests ported (`test/pnpm_install_misc.bats`, 27 → 30 of 36)

337 workspace per-project engines.node strict fail
371 workspace per-project engines.node warn
303 workspace per-project engines.pnpm (translated: pnpm hard-fails this unconditionally; aube routes everything through one knob, so the port enables engine-strict=true)

Doc reclassification

misc.ts:136 (package.yaml manifest) moves from "documented divergence" to won't fix. Supporting YAML manifests would expand both the parser surface and the security-review surface for a feature pnpm itself documents as legacy. Not a pnpm-parity goal.

Test plan

13 unit tests in crates/aube/src/engines.rs (8 new — check_root_flags_{aube,pnpm}_mismatch, check_root_aube_satisfied_when_range_matches, check_root_flags_all_three_engines_independently, check_workspace_importers_{skips_root,flags_per_project,falls_back_to_rel_path_label})
30/30 tests in test/pnpm_install_misc.bats pass
5/5 tests in test/engines.bats (existing) still pass
65/65 tests in test/install.bats still pass
18/18 tests in test/workspace.bats still pass
All 354 unit tests via cargo test --bin aube pass
cargo clippy --all-targets -- -D warnings clean
cargo fmt --check clean

🤖 Generated with Claude Code

Note

Medium Risk
Changes dependency resolution to prefer dist-tags.latest within a semver range and expands engine constraint enforcement across workspaces, which can alter installed versions and cause new install failures under engine-strict. Limited scope but touches core install/resolution paths.

Overview
Aligns resolver and CLI output with npm/pnpm by preferring dist-tags.latest when it satisfies a semver range (unless pick_lowest), and adds tests covering the new selection behavior.

Extends install-time engine validation to check engines.aube/engines.pnpm (against CARGO_PKG_VERSION) and to apply engines.node checks to workspace importer manifests as well as transitive deps; warnings/errors now label the specific engine key and avoid falsely failing engines.node when no Node version is available.

Adds pnpm parity BATS tests for workspace engine-strict behavior and updates pnpm import docs to reflect the newly covered cases.

^{Reviewed by Cursor Bugbot for commit d631f88. Bugbot is set up for automated code reviews on this repo. Configure here.}

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 9de27dd. Configure here.}

greptile-apps · 2026-05-01T15:59:43Z

Greptile Summary

This PR extends install-time engine validation to cover engines.aube and engines.pnpm (checked against env!(\"CARGO_PKG_VERSION\")), applies engines.node checks to each workspace-project manifest (not just the root and transitive deps), and fixes the prior sentinel-string approach by threading Option<&str> for node_version. A parallel change adds dist-tags.latest preference to pick_version and mirrors it in update_manifest_for_add's highest_satisfying closure.

The only notable gap is in add.rs: the mirrored latest preference omits the passes_cutoff time-gate that pick_version applies, so aube add can pin a version in the manifest that aube install would not resolve in time-based mode.

Confidence Score: 5/5

Safe to merge; the single P2 finding is a corner-case divergence in time-based mode that does not affect the main engines or install paths

All issues from previous reviews are addressed in this diff. The only new finding is a P2 (missing passes_cutoff in add.rs) that only manifests in time-based resolution mode with a latest tag newer than the cutoff. The core engines.rs refactor is well-tested with 13 unit tests and three new integration tests.

crates/aube/src/commands/add.rs — highest_satisfying closure is missing the passes_cutoff time-gate

Important Files Changed

Filename	Overview
crates/aube/src/engines.rs	Major refactor: adds Engine enum, check_manifest_engines, check_workspace_importers; threads Option<&str> for node_version to skip engines.node when Node is absent; 13 unit tests cover all new paths
crates/aube-resolver/src/semver_util.rs	New dist-tags.latest preference in pick_version: returns the publisher-tagged build when it satisfies the range and passes the time-based cutoff; skipped in pick_lowest mode
crates/aube/src/commands/add.rs	Mirrors pick_version's dist-tags.latest preference in highest_satisfying, but is missing the passes_cutoff time-gate and !pick_lowest guard present in the resolver
crates/aube/src/commands/install/mod.rs	One-liner: passes &manifests to run_checks to enable workspace-importer engine validation
crates/aube-resolver/src/tests.rs	Four new unit tests covering latest-in-range preference, latest-outside-range fallthrough, and pick_lowest suppression
test/pnpm_install_misc.bats	Three new integration tests for workspace per-project engines.node strict/warn and engines.pnpm strict failure; all well-isolated with node-version overrides
test/PNPM_TEST_IMPORT.md	Tracking doc updated: 27→30 ported tests, package.yaml moved from documented-divergence to won't-fix, engines.pnpm and workspace-engines entries moved to Done

_{Reviews (4): Last reviewed commit: "fix(resolver): prefer dist-tags.latest w..." | Re-trigger Greptile}

github-actions · 2026-05-01T16:04:27Z

Benchmark changes

Versions:

aube: 1.5.2 -> 1.6.1

Public ratios: warm installs vs Bun 6x -> 7x; warm installs vs pnpm 10x -> 13x.

Benchmark	aube	bun	pnpm
Fresh install (warm cache)	230ms -> 252ms (+10%)	1488ms -> 1816ms (+22%)	2367ms -> 3162ms (+34%)
CI install (warm cache, GVS disabled)	564ms -> 866ms (+54%)	1295ms -> 1782ms (+38%)	2361ms -> 2360ms (0%)
CI install (cold cache, GVS disabled)	5800ms -> 4311ms (-26%)	4278ms -> 4294ms (+0%)	4823ms -> 5878ms (+22%)

Three review fixes on PR #458: 1. P1: `run_checks` previously fed a sentinel string `"0.0.0-no-node"` into the manifest helpers when no Node was probed. The sentinel is legal SemVer (prerelease tag `no-node`), so `version_satisfies` did *not* take the parse-failure "no opinion" fallback — it ran for real against e.g. `>=20` and reported a spurious mismatch on every manifest declaring `engines.node`. Under engine-strict that hard-failed installs on machines without Node. Thread `Option<&str>` through `check_root` / `check_workspace_importers` / `check_manifest_engines` and skip the field when None. 2. P1: restore the `geteuid() == 0` early-return in `collect_dep_mismatches_propagates_non_not_found_io_errors`. Docker CI defaults to uid 0; root ignores `chmod 000`, so the permission-denied read would succeed and the assertion would panic. 3. P2: re-add the `resolve_node_version_strips_v_prefix` unit test that was lost in the refactor. The strip path is the most common real-world input (`node --version` always emits `vX.Y.Z`) and deserves a dedicated regression guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Extends `engines::run_checks` to cover constraints aube previously ignored: - `engines.aube` and `engines.pnpm` are now validated against aube's own version (`env!("CARGO_PKG_VERSION")`). aube positions itself as a pnpm-compatible drop-in, so a package gating on `engines.pnpm` is honored as if aube were that pnpm. Both fields are checked on the root manifest and workspace-project manifests; transitive deps stay `engines.node`-only to avoid drowning users in warnings from wild packages that pin authors' own toolchains. - Workspace per-project manifests are now walked for `engines.{node, aube,pnpm}` mismatches. Previously only the root manifest was checked; a sub-project pinning `engines.node: ">=99999"` silently passed. They warn by default and hard-fail under `engine-strict`. The warning printer now labels each mismatch with which engine field triggered it (`wanted node >=20`, `wanted aube >=99999`, etc.). The strict-mode error is now engine-agnostic ("declare incompatible engine constraints" instead of "require a Node version incompatible with"). Existing engines.bats assertions on the "engine-strict" substring keep working. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds three engines-coverage ports enabled by the new check_engines/ workspace-importer-walk plumbing: - 337 — workspace per-project engines.node fails install under engine-strict. - 371 — same fixture without engine-strict warns and succeeds. - 303 — workspace per-project engines.pnpm fails install under engine-strict. Pnpm fails this unconditionally; aube routes everything through one knob, so the port enables engine-strict. Reclassifies misc.ts:136 (package.yaml manifest) from "documented divergence" to "won't fix" — supporting YAML manifests would expand both the parser and the security-review surface for a feature pnpm itself documents as legacy. Not a pnpm-parity goal. misc.ts ported count: 27 -> 30 of 36. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Three review fixes on PR #458: 1. P1: `run_checks` previously fed a sentinel string `"0.0.0-no-node"` into the manifest helpers when no Node was probed. The sentinel is legal SemVer (prerelease tag `no-node`), so `version_satisfies` did *not* take the parse-failure "no opinion" fallback — it ran for real against e.g. `>=20` and reported a spurious mismatch on every manifest declaring `engines.node`. Under engine-strict that hard-failed installs on machines without Node. Thread `Option<&str>` through `check_root` / `check_workspace_importers` / `check_manifest_engines` and skip the field when None. 2. P1: restore the `geteuid() == 0` early-return in `collect_dep_mismatches_propagates_non_not_found_io_errors`. Docker CI defaults to uid 0; root ignores `chmod 000`, so the permission-denied read would succeed and the assertion would panic. 3. P2: re-add the `resolve_node_version_strips_v_prefix` unit test that was lost in the refactor. The strip path is the most common real-world input (`node --version` always emits `vX.Y.Z`) and deserves a dedicated regression guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

aube's pick_version always returned the strictly-highest version satisfying the range. npm and pnpm both prefer the version the publisher last tagged `latest` when it falls inside the range — a stray higher version (hotfix on an old line, withdrawn experimental publish, mid-rollback intermediary) shouldn't silently win over the publisher's anchor. The pnpm_update.bats serial cases (`add_dist_tag latest 100.0.0 + aube add foo@^100.0.0` -> expect lockfile pinned 100.0.0) bake this behavior in. Without the preference, those 4 tests were a flaky CI failure that finally went red on c4ac6a6 — a workflow-only commit, ruling out any actual regression. Apply the same preference in `aube add`'s display-version path so the printed `+ pkg@version` line agrees with what the resolver installs. TimeBased mode (`pick_lowest=true`) suppresses the preference: that mode pins to the floor of the range by design. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cursor Bot reviewed May 1, 2026

View reviewed changes

Comment thread crates/aube/src/engines.rs Outdated

Comment thread crates/aube/src/engines.rs

greptile-apps Bot reviewed May 1, 2026

View reviewed changes

Comment thread crates/aube/src/engines.rs Outdated

Comment thread crates/aube/src/engines.rs

jdx and others added 3 commits May 1, 2026 21:43

jdx force-pushed the claude/engines-pnpm-and-workspace branch from b44605c to 6274303 Compare May 1, 2026 21:50

jdx enabled auto-merge (squash) May 1, 2026 22:02

jdx merged commit 3f67c94 into main May 1, 2026
18 checks passed

jdx deleted the claude/engines-pnpm-and-workspace branch May 1, 2026 22:06

greptile-apps Bot mentioned this pull request May 1, 2026

chore: release v1.6.2 #465

Merged

jdx changed the title ~~feat(cli): check engines.{aube,pnpm} and workspace per-project engines~~ fix(cli): validate engines.{aube,pnpm} + workspace engines.node + dist-tag preference May 1, 2026

greptile-apps Bot mentioned this pull request May 1, 2026

chore: release v1.7.0 #466

Merged

chenrui333 mentioned this pull request May 2, 2026

aube 1.6.2 chenrui333/homebrew-tap#6587

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cli): validate engines.{aube,pnpm} + workspace engines.node + dist-tag preference#458

fix(cli): validate engines.{aube,pnpm} + workspace engines.node + dist-tag preference#458
jdx merged 4 commits intomainfrom
claude/engines-pnpm-and-workspace

jdx commented May 1, 2026 •

edited by cursor Bot

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented May 1, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 1, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jdx commented May 1, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Engines coverage changes

Tests ported (test/pnpm_install_misc.bats, 27 → 30 of 36)

Doc reclassification

Test plan

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented May 1, 2026 •

edited by cursor Bot

Loading

Tests ported (`test/pnpm_install_misc.bats`, 27 → 30 of 36)

greptile-apps Bot commented May 1, 2026 •

edited

Loading

github-actions Bot commented May 1, 2026 •

edited

Loading