fix(lockfile): a declaration resolves ambiguity, whatever it names - #647
Conversation
…nnot help A declaration naming nub is a self-name: the engine reads it as "preserve whatever format this project already uses", which settles nothing when two candidate lockfiles are present. Since `nub install` writes that declaration itself, telling an ambiguous project to declare its manager sent the common case in a circle. Splits the shared remedy so the ambiguity error names an action that actually resolves it, and leaves the declaration-mismatch remedy unchanged, where setting the declaration is still correct. Corrects two docs claims that said the same untrue thing, and replaces the mockup's invented help line with real captured output. Also drops a warn callout on the Cloudflare page claiming .tool-versions is unsupported there. It is supported: Workers Builds announces it, provisions a pinned nub version on demand, and nub reads the same file. Documents SKIP_DEPENDENCY_INSTALL, which is how a nub-owned project avoids the second lockfile the builder's own install creates. Refs #473
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adjusts Nub’s lockfile-ambiguity guidance so nub install no longer tells nub-declared projects to “set a declaration” that can’t resolve the ambiguity, and updates docs to reflect the corrected remedy (including Cloudflare Workers Builds guidance).
Changes:
- Split the lockfile-identity error help text so ambiguity and declaration-mismatch have different (correct) remedies.
- Added a regression test covering the nub-declared + multiple-lockfile ambiguity case.
- Updated install + Cloudflare deployment docs to remove the misleading declaration advice and document
SKIP_DEPENDENCY_INSTALL.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| site/content/docs/install/index.mdx | Updates ambiguity explanation and captured error output to avoid recommending a nub self-declaration as a fix. |
| site/content/docs/deployment/cloudflare.mdx | Documents Cloudflare Workers Builds behavior, SKIP_DEPENDENCY_INSTALL, and version pinning via .tool-versions. |
| crates/nub-cli/tests/pm_identity.rs | Adds regression test ensuring ambiguity help doesn’t advise “set the declaration” for nub-declared projects. |
| crates/nub-cli/src/pm_engine/mod.rs | Splits ambiguity vs mismatch remedies in identity_error help text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **lockfile on disk** | ||
|
|
||
| In a workspace, the chain runs from any member: Nub walks up to the root, which carries the declaration and lockfile. Two lockfiles for different managers, with no declaration, is a hard error. | ||
| In a workspace, the chain runs from any member: Nub walks up to the root, which carries the declaration and lockfile. Two lockfiles for different managers is a hard error unless a declaration names one of them. Declaring Nub itself does not settle it: Nub preserves whatever format the project already uses, so with two candidates there is nothing to choose between. |
| nodejs 24.18.0 | ||
| nub 0.6.0 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The Rust half is sound, but the Cloudflare page's new .tool-versions pinning advice replaces an accurate warning with claims no published source supports — and the sub-claim that an asdf short name nub resolves is provably not in place yet.
Reviewed changes — the full diff at c4bb338: one Rust behavior change, one new integration test, and two docs pages.
- Split remedy string —
identity_errornow carries a separateAMBIGUITY_REMEDY(remove the stale lockfile, or run nub pm use <pm> naming a specific manager) whileDeclarationMismatchkeeps the old wording. No control-flow change. - Premise verified —
detect.rs:230filters the declaration throughis_self_name, and nub's profile setsself_names: &["nub"]pluscanonical_lockfile_always_wins: false(identity.rs:155,185), so a nub-declared project with two lockfile families really does land onAmbiguousLockfiles.nub installdoes stamp that declaration itself (install_family.rs:1218). - Regression test can fail — the new test negates
"set the declaration", which the pre-fix sharedREMEDYcontained, so it pins the actual behavior rather than passing vacuously. - Install docs — the corrected ambiguity claim and the nub-self-name caveat match the engine's decision table.
- Cloudflare docs —
SKIP_DEPENDENCY_INSTALLchecks out against Cloudflare's build-image page (1 or true). The.tool-versions/ asdf section does not; see the inline comments.
ℹ️ The long-term answer for the hosted-builder state is still open
The message is better, but the state itself remains a hard error by design: nub's profile deliberately sets canonical_lockfile_always_wins: false, so a committed nub.lock beside a builder-written bun.lock cannot resolve on its own. This PR routes users to a build variable, which works only on builders that expose one. Worth deciding explicitly whether the build-var workaround is the permanent story, or whether nub.lock beside exactly one foreign lockfile should eventually resolve to nub.lock — that call belongs to you, not to a follow-up review.
ℓ Nitpicks
site/content/docs/pm/index.mdx:24still offers "declare the pin with [nub pm use]" as the ambiguity fix, without the nub-self-name caveat this PR adds toinstall/index.mdx. Same claim, one page over.
Claude Opus ｜ 𝕏
| Workers Builds provisions its tools with asdf, so a project-root `.tool-versions` pins Nub and Node together: | ||
|
|
||
| ```text title=".tool-versions" | ||
| nodejs 24.18.0 | ||
| nub 0.6.0 | ||
| ``` | ||
|
|
||
| A pinned version the image doesn't already carry is downloaded during the build, so the pin holds even when it differs from the image default. Nub reads the same file when resolving its own Node, which keeps the two in agreement from one place. |
There was a problem hiding this comment.
No published source supports this, and one sub-claim is refutable today: the asdf short name nub does not resolve — asdf-vm/asdf-plugins/plugins/nub 404s and the registry PR is still open (per #473) — so nub 0.6.0 only works if Cloudflare wired the plugin into its own fixed set. Cloudflare's build-image page documents version files for .nvmrc/.node-version/.python-version/.ruby-version only, never .tool-versions, and lists no nub row at all.
Technical details
# Cloudflare `.tool-versions` pinning is asserted, not verified
## Affected sites
- `site/content/docs/deployment/cloudflare.mdx:98-105` — asserts asdf provisioning, a `nub 0.6.0` pin, and "a pinned version the image doesn't already carry is downloaded during the build".
- `site/content/docs/deployment/cloudflare.mdx:22` — adds `.tool-versions` to the Node-version source list for this image.
- The deleted `<Callout type="warn">` said Workers Builds does not document `.tool-versions`, which still matches both Cloudflare's page and #473's own wording ("not `.tool-versions`, and there's no plugin mechanism").
## Evidence
- https://developers.cloudflare.com/workers/ci-cd/builds/build-image/ (updated 2026-07-30): Runtime table's File column is `.nvmrc, .node-version` for Node.js; the Tools-and-languages table has no File column at all; "asdf" appears nowhere; nub appears nowhere. `SKIP_DEPENDENCY_INSTALL` = `1 or true` IS documented, so the earlier section in this diff is fine.
- `https://raw.githubusercontent.com/asdf-vm/asdf-plugins/master/plugins/nub` → 404. Registry PR `asdf-vm/asdf-plugins#1170` still open.
- https://www.codejam.info/2026/02/cloudflare-workers-choke-asdf-tool-versions.html (2026-02-10): Workers/Pages read `.tool-versions` undocumented and fail on a bare `nodejs 24.13.1` line (`Failed: error occurred while installing tools or dependencies`), and reports `SKIP_DEPENDENCY_INSTALL` does not suppress that handling — which interacts with this diff's other new section, since a reader following both adds a `.tool-versions` AND sets that variable.
- Counter-evidence: #473 carries a Cloudflare engineer's note that nub was being added to the image's plugin set around 2026-07-30, so the claim may well be true in practice. #473's own step 3 required confirming `.tool-versions` provisioning end-to-end on a real Workers build BEFORE documenting it.
## Required outcome
Either cite/perform the real Workers Builds run that proves a `.tool-versions` `nub` pin provisions the requested version, or drop the section back to the devDependency pin and keep a caveat that `.tool-versions` is undocumented on this image.| Cloudflare runs an optional build command and then a Wrangler deploy command. Point the build command at the package script that uses Nub; the preinstalled binary means no devDependency is required. | ||
|
|
||
| The image currently defaults to Node 24.18.0 and also preinstalls Node 22.23.2. Use `.node-version` or `.nvmrc` to select a Node version that both Cloudflare and Nub read. | ||
| The image currently defaults to Node 24.18.0 and also preinstalls Node 22.23.2. Use `.node-version`, `.nvmrc`, or `.tool-versions` to select a version that both Cloudflare and Nub read. |
There was a problem hiding this comment.
Same evidence as above: Cloudflare's build-image page lists .nvmrc and .node-version as the Node version files and never mentions .tool-versions, so "a version that both Cloudflare and Nub read" holds for the first two but not the third. Nub reading .tool-versions (nub-core/src/node/discovery.rs:499) is not in question — Cloudflare's side is.
| const MISMATCH_REMEDY: &str = | ||
| "set the declaration: nub pm use <pm> — or remove the stale lockfile"; | ||
| const AMBIGUITY_REMEDY: &str = | ||
| "remove the stale lockfile, or run nub pm use <pm> naming a specific manager"; |
There was a problem hiding this comment.
"naming a specific manager" is broader than what works: plan_alignment only clears the ambiguity when that manager's lockfile is already on disk (use_align.rs:114-131), and otherwise bails at use_align.rs:148 with its own remove-the-stale-ones-first message. In this PR's own new fixture (package-lock.json + yarn.lock), nub pm use nub is one of the targets that bails. Scoping the wording to a manager that owns one of the lockfiles found would make the second clause unconditionally true.
| "remove the stale lockfile, or run nub pm use <pm> naming a specific manager"; | |
| const AMBIGUITY_REMEDY: &str = | |
| "remove the stale lockfile, or run nub pm use <pm> naming one of the managers found"; |
The module docs for detect.rs have always promised that a declaration naming the running tool "never makes a multi-lockfile project ambiguous". The code 230 lines below said the opposite and won, so a declared project with two lockfiles refused to install. That made the state inescapable rather than merely strict: `nub install` writes `devEngines.packageManager: nub` itself, so once anything dropped a second lockfile beside nub.lock — a hosted builder running its own install is the common case — every later install refused, and the suggested remedy was already applied. A self-name declaration now resolves like any other: the canonical lockfile wins when present, else the highest-precedence candidate. An undeclared project with two lockfiles is still a hard error, unchanged. Note this exposes a stray lockfile to parsing that the ambiguity check previously short-circuited: a malformed stray now fails on its own merits rather than as an identity error. Refs #473
colinhacks
changed the title
There was a problem hiding this comment.
Caution
a5cca4a turns the message fix into a behavior change, and the state it makes reachable is one every lockfile reader still answers differently. In a nub-declared project with nub.lock beside a foreign lockfile, nub install now writes nub.lock while reading the foreign one — under --frozen-lockfile it installs the stray's graph outright. That is the exact Cloudflare state the PR is about, so a loud error becomes a silent wrong install.
Reviewed changes — the delta from c4bb338 to a5cca4a. The Rust remedy strings, both docs pages, and the three open threads from the prior review are untouched by this commit.
- Made a self-name declaration resolve a multi-lockfile project —
detect.rs's undeclared branch now rescuesErr(AmbiguousLockfiles)whenever a declaration is present, returning the canonical lockfile when it exists and the highest-precedence candidate otherwise. - Documented a new decision-table row — module docs gained
| self | two or more PMs' | Existing(canonical, else precedence) |plus the claim that only an undeclared project is ambiguous. - Added two
detect.rsunit tests — canonical-wins, and the no-canonical precedence fallback. - Repurposed the
pm_identity.rsregression test —a_nub_declared_ambiguous_project_is_not_told_to_set_a_declarationbecamea_nub_declared_project_resolves_past_a_stray_lockfile, now asserting exit 0 onnub.lock+ a realbun.lock.
⚠️ canonical_lockfile_always_wins: false is now half-true, and which half survives is your call
The NUB profile's own docs say nub.lock "never silently outranks a foreign lockfile beside it; that state is the loud ambiguity/contradiction error" (identity.rs:102-105), and lock_yaml_is_nub_identity_and_conflicts_are_loud's docstring says the same. After this commit that holds only for undeclared projects — and since nub install stamps the declaration itself, the declared case is the common one. Two end states are coherent; the middle one this commit lands on is the only one that splits the resolver's answer from every reader's.
Technical details
# The strict-identity posture needs an explicit decision, not a partial exemption
## Affected sites
- `crates/nub-cli/src/pm_engine/identity.rs:102-105` — the profile doc for `canonical_lockfile_always_wins` states the opposite of the new resolver behavior.
- `crates/nub-cli/tests/pm_identity.rs:493-498` — the same claim in a test docstring ("never a silent winner").
- `vendor/aube/crates/aube-lockfile/src/detect.rs:68-80` — `canonical_lockfile_always_wins`'s own doc comment describes the flag as making the state "the loud `Error::AmbiguousLockfiles` / `Error::DeclarationMismatch` instead of a silent win".
## Required outcome
- One documented posture that the resolver, the raw candidate order, and these three doc sites all agree on.
## Suggested approach (optional)
- **Keep strict identity:** leave the flag `false`, keep the declared-plus-stray state loud, and address the hosted-builder case at the build level only (`SKIP_DEPENDENCY_INSTALL`, which the Cloudflare page already documents).
- **Or flip the flag to `true`:** `lockfile_candidates` then leads with the canonical entries, so the raw readers and the declaration-aware resolver agree with no threading work, and this new arm is largely redundant. The cost is that the *undeclared* `nub.lock` + foreign state stops being loud, which `pm_identity.rs:514-529` currently pins.
## Open questions for the human
- Should the resolved-past lockfile be named in a `WARN_NUB_*`? Today it is ignored in silence, and on a hosted builder it is regenerated on every build, so nothing ever tells the user the file is inert.ℹ️ Nitpicks
a_self_name_declaration_without_the_canonical_file_falls_to_precedenceasserts onlymatches!(…, Existing(_)). The test names precedence, soassert_eq!(resolve(&d).unwrap(), ResolvedLockfileKind::Existing(LockfileKind::Pnpm))would actually pin it — moot if the fallback branch goes away.
Claude Opus ｜ 𝕏
| // running its own install is the common case — every later install | ||
| // refused, and the suggested remedy (declare a manager) was | ||
| // already done. | ||
| Err(Error::AmbiguousLockfiles { found }) if declared.is_some() => { |
There was a problem hiding this comment.
This makes a state reachable that every lockfile reader still answers differently. resolve_project_lockfile_kind returns Existing(Aube) here, but parse_lockfile_with_kind_and_options walks lockfile_candidates, which puts the canonical entries last under canonical_lockfile_always_wins: false — so under nub the first existing candidate is the foreign lockfile. nub install therefore writes nub.lock and reads bun.lock.
Under --frozen-lockfile (the CI default) that installs the builder's graph and drift-checks against it; in the default mode it seeds the resolver from it and stamps those pins into nub.lock. The committed lockfile is ignored either way.
Technical details
# The declaration-aware resolver and the raw lockfile readers disagree in the newly-reachable state
## Affected sites
- `vendor/aube/crates/aube-lockfile/src/detect.rs:277-290` — returns `Existing(Aube)` for self-name declaration + `nub.lock` + foreign lockfile. Previously `Err(AmbiguousLockfiles)`, so the state aborted before any reader ran.
- `vendor/aube/crates/aube-lockfile/src/io.rs:590-603` — `lockfile_candidates` appends the Aube entries AFTER the foreign ones when `canonical_lockfile_always_wins == false` (`crates/nub-cli/src/pm_engine/identity.rs:185`).
- `vendor/aube/crates/aube-lockfile/src/io.rs:495-510` — `parse_lockfile_with_kind_and_options` returns the first existing candidate: the foreign lockfile.
- `vendor/aube/crates/aube/src/commands/install/resolve.rs:504` — `FrozenMode::Frozen` parses through that path, so `check_drift_workspace_for_kind` and the installed graph both come from the foreign lockfile.
- `vendor/aube/crates/aube/src/commands/install/resolve.rs:30` — `pre_parse_lockfile` feeds `existing_for_resolver` (`install/mod.rs:1102-1107`) in the default Prefer mode, so the stray's pins are reused and written into `nub.lock` (write target from `install/mod.rs:1041` → `settings_context.rs:473`, which IS declaration-aware).
- `vendor/aube/crates/aube-lockfile/src/io.rs:341-351` — `active_lockfile_has_conflict_markers` inspects the foreign file, so a real conflict left in `nub.lock` is missed.
- Same raw primitive, further consequences: `install/workspace.rs:428` (`sharedWorkspaceLockfile=false` writes a member lockfile in the FOREIGN format), `add/supply_chain.rs:64-81` (allowlist built from the stray's package names), `commands/fetch.rs:53`, `runtime.rs:754-773` (`devEngines.runtime` pin silently dropped), `install/settings.rs:702-717` (`packageExtensionsChecksum` not stamped, which a later frozen run expects).
## Required outcome
- In the resolved state, the file the install READS is the same file it writes — for the frozen path, the resolver-reuse baseline, and conflict-marker detection alike.
- The behavior is pinned by a test that fails when the two disagree.
## Suggested approach (optional)
- Thread the already-computed resolved kind into the read path (a `parse_lockfile_of_kind`-style entry point, or pass `source_kind_before` into `pre_parse_lockfile` and the `FrozenMode::Frozen` parse) so the declaration governs reading as well as writing.
- Or flip `canonical_lockfile_always_wins` to `true` for the NUB profile, which reorders `lockfile_candidates` itself and makes both sides agree without threading — see the review body for what that costs.| match existing | ||
| .iter() | ||
| .find(|(_, k)| *k == LockfileKind::Aube) | ||
| .or_else(|| existing.first()) | ||
| { | ||
| Some((path, kind)) => Ok(ResolvedLockfileKind::Existing(refine_yarn_kind( | ||
| path, *kind, | ||
| ))), | ||
| // Unreachable in practice (an ambiguity implies at least | ||
| // two candidates); propagate rather than panic. | ||
| None => Err(Error::AmbiguousLockfiles { found }), | ||
| } |
There was a problem hiding this comment.
.or_else(|| existing.first()) picks between two foreign families by filename precedence. A self-name declaration means "preserve whatever format this project already uses" — with pnpm-lock.yaml and yarn.lock both present there is no such format, which is what the comment deleted just above this said. The hosted-builder state that motivates the PR always has the canonical file, so nothing in scope needs this branch.
| match existing | |
| .iter() | |
| .find(|(_, k)| *k == LockfileKind::Aube) | |
| .or_else(|| existing.first()) | |
| { | |
| Some((path, kind)) => Ok(ResolvedLockfileKind::Existing(refine_yarn_kind( | |
| path, *kind, | |
| ))), | |
| // Unreachable in practice (an ambiguity implies at least | |
| // two candidates); propagate rather than panic. | |
| None => Err(Error::AmbiguousLockfiles { found }), | |
| } | |
| match existing.iter().find(|(_, k)| *k == LockfileKind::Aube) { | |
| Some((path, kind)) => Ok(ResolvedLockfileKind::Existing(refine_yarn_kind( | |
| path, *kind, | |
| ))), | |
| // No canonical lockfile among the candidates: a self-name | |
| // declaration accepts every format aube can preserve, so | |
| // with two there is no fact of the matter which one. | |
| None => Err(Error::AmbiguousLockfiles { found }), | |
| } |
| //! | self | two or more PMs' | `Existing(canonical, else precedence)` | | ||
| //! | ||
| //! The last row is why a declaration is worth setting: only an | ||
| //! *undeclared* project is ambiguous. A declaration — whether it names |
There was a problem hiding this comment.
"always resolves" overstates it. A declaration naming a tool declared_lockfile_kind doesn't know (vlt, deno) passes the is_self_name filter at line 235, gets None, and goes straight to resolve_undeclared at line 257 — so it still returns AmbiguousLockfiles, with the new arm never consulted. Worth qualifying to self-names and known managers, since a docs-versus-code mismatch in this very paragraph is what the PR set out to fix.
| /// The two states get DIFFERENT remedies because "set the declaration" is a | ||
| /// dead end for one of them. A declaration naming nub is a self-name: the | ||
| /// engine reads it as "preserve whatever format this project already uses" | ||
| /// (`aube-lockfile`'s `is_self_name` carve-out), which resolves nothing when |
There was a problem hiding this comment.
"which resolves nothing when two candidate lockfiles are present" is precisely what a5cca4a reversed, so this paragraph now documents the pre-fix engine. The remedy split still earns its keep — ERR_NUB_LOCKFILE_AMBIGUOUS can now only reach an undeclared or foreign-declared project — but the stated reason needs rewriting to that.
| - **lockfile on disk** | ||
|
|
||
| In a workspace, the chain runs from any member: Nub walks up to the root, which carries the declaration and lockfile. Two lockfiles for different managers, with no declaration, is a hard error. | ||
| In a workspace, the chain runs from any member: Nub walks up to the root, which carries the declaration and lockfile. Two lockfiles for different managers is a hard error unless a declaration names one of them. Declaring Nub itself does not settle it: Nub preserves whatever format the project already uses, so with two candidates there is nothing to choose between. |
There was a problem hiding this comment.
"Declaring Nub itself does not settle it" is now the opposite of the engine's behavior: detect.rs:277 resolves a self-name declaration to nub.lock, else to the highest-precedence candidate. Line 215's "Two lockfiles it cannot choose between" lead-in and line 227's hosted-builder sentence carry the same stale claim — the ambiguity error now requires no declaration.
| </Callout> | ||
| ## Letting Nub run the install | ||
|
|
||
| Workers Builds installs dependencies itself before the build command runs, picking the package manager from the lockfile it finds. It does not yet recognize `nub.lock`, so a Nub-owned project ends up with a second lockfile from whichever manager Cloudflare falls back to. The next `nub install` then refuses, unable to tell which one owns the project. |
There was a problem hiding this comment.
"The next nub install then refuses" no longer happens for a Nub-owned project: that project carries the devEngines.packageManager: nub declaration nub install stamps, which the new resolver arm resolves. SKIP_DEPENDENCY_INSTALL is still the right advice — the reason is now that Cloudflare's install is wasted work and leaves a stray lockfile behind that its next build picks up again.
| "a declaration resolves ownership — no ambiguity: {stderr}" | ||
| ); | ||
| assert!( | ||
| dir.join("nub.lock").is_file(), |
There was a problem hiding this comment.
nub.lock is a file because the test wrote it two lines up, and nothing on this path deletes it — the assertion holds even if the install resolved against bun.lock. Together with assert_eq!(code, 0) that means the test passes under the read/write divergence flagged in detect.rs. Giving the two lockfiles different pins for one dependency and asserting the installed version came from nub.lock would make it able to fail.
|
Shipped in v0.7.0: https://github.com/nubjs/nub/releases/tag/v0.7.0 |
Two changes to the same error.
Behavior.
detect.rs's module docs have always promised a declaration naming the running tool "never makes a multi-lockfile project ambiguous". The code contradicted them, so a declared project with two lockfiles refused to install. That was inescapable, not just strict:nub installwritesdevEngines.packageManager: nubitself, so any stray second lockfile broke every later install and the suggested remedy was already applied. A self-name declaration now resolves like any other — canonical lockfile wins, else precedence. Undeclared with two lockfiles is still a hard error, unchanged.Message. The ambiguity remedy no longer advises setting a declaration; declaration-mismatch keeps its wording, where the advice is correct.
Found on Cloudflare Workers Builds, which manufactures the state by running its own install (bun) beside a committed
nub.lock.Docs: corrects two claims that a declaration resolves ambiguity, replaces an invented mockup line with captured output, drops a false
.tool-versionswarning on the Cloudflare page, and documentsSKIP_DEPENDENCY_INSTALL.Caveat: resolving past a stray exposes it to parsing, so a malformed stray now fails on its own merits rather than as an identity error.
Refs #473