Repository navigation
fix(install): repair member installs under sharedWorkspaceLockfile=false - #891
Conversation
Under sharedWorkspaceLockfile=false a workspace writes one lockfile per member and no shared root lockfile. The warm path anchored its freshness decision on a single root lockfile, so a config-only root always read "no lockfile found" and fell through to the full resolve/fetch/delta pipeline on every aube install (and every aube run auto-install check), re-walking the whole graph and visibly re-linking even when nothing changed. Record each member's lockfile fingerprint in the install state and, when no root lockfile is present, verify the per-member lockfiles instead of bailing. Re-enumerating members on each check keeps added, removed, and edited members busting the warm path. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements two independent features: (1) ChangespackageExtensions for non-registry packages
sharedWorkspaceLockfile=false warm path and workspace root discovery
Sequence DiagramsequenceDiagram
participant cli as aube install
participant check as check_needs_install_inner
participant mlf as member_lockfiles_stale
participant merge as merge_member_lockfile_graphs
participant linker as Linker
cli->>check: check freshness (FreshnessState)
check->>check: compare lockfile hash (layout transition handled)
check->>mlf: compare per-member lockfile hashes + mtime
mlf-->>check: stale reason / None
check-->>cli: NeedsInstall or AlreadyUpToDate
alt NeedsInstall and sharedWorkspaceLockfile=false
cli->>merge: merge member per-project lockfiles into graph
merge-->>cli: graph with all importer entries
cli->>linker: link with full importer graph
end
cli->>cli: write_state (collect_member_lockfile_state)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Greptile SummaryThis PR fixes four bugs affecting workspace members under
Confidence Score: 5/5Safe to merge; the four targeted bug fixes are well-reasoned, each has dedicated unit and/or bats integration tests, and no new correctness gaps were introduced in the changed code paths. All four fix areas are covered by new tests and the logic is sound. The find_workspace_yaml_root gap affecting aube add catalog resolution is a pre-existing issue already flagged before this PR and not made worse here. No new regressions were found in the reviewed code. crates/aube/src/dirs.rs — find_workspace_yaml_root still uses workspace_yaml_existing and will stop at a settings-only member yaml, so aube add from such a member still resolves catalogs against the member directory instead of the real workspace root. Important Files Changed
Reviews (4): Last reviewed commit: "fix(workspace): stop root discovery from..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aube-manifest/src/workspace/config.rs`:
- Around line 816-819: The current implementation collapses unparseable or
unreadable nested pnpm-workspace.yaml files into
WorkspaceYamlKind::SettingsOnly, allowing discovery to continue walking up the
tree. Instead of collapsing the error case at the Err(_) arm in the match
expression, create or use a distinct WorkspaceYamlKind variant that explicitly
represents an invalid or broken YAML state. This distinct state should signal to
stop discovery at that location rather than silently continuing to resolve to a
parent workspace, ensuring parsing and I/O errors are properly surfaced instead
of hidden.
In `@crates/aube/src/state.rs`:
- Around line 325-347: In the `if let Some(path) = lockfile_path` branch, the
code only verifies the root lockfile hash but does not check if member lockfiles
have drifted. In a `sharedWorkspaceLockfile=false` workspace that has both a
root `package.json` and per-member lockfiles, you must also verify member
lockfiles are up-to-date by calling `member_lockfiles_stale(project_dir,
&state)` in addition to checking the root lockfile hash. If the root lockfile
hasn't changed but a member lockfile has drifted, the check will incorrectly
allow the warm path to proceed. Add a check for `member_lockfiles_stale` so that
any member lockfile changes are detected even when the root lockfile remains
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3471f7e0-4ad9-43d9-93df-5e8e96dae67a
📒 Files selected for processing (15)
crates/aube-lockfile/src/pnpm/tests.rscrates/aube-manifest/src/workspace.rscrates/aube-manifest/src/workspace/config.rscrates/aube-resolver/src/lib.rscrates/aube-resolver/src/package_ext.rscrates/aube-resolver/src/resolve/driver.rscrates/aube-resolver/src/tests.rscrates/aube-store/src/lib.rscrates/aube/src/commands/install/mod.rscrates/aube/src/commands/install/workspace.rscrates/aube/src/dirs.rscrates/aube/src/state.rstest/filter.batstest/git_deps.batstest/shared_workspace_lockfile_warm_path.bats
Add WorkspaceYamlKind::Invalid for a present-but-broken workspace yaml. find_workspace_root now anchors on the member's own directory instead of silently escaping to an enclosing workspace, so the parse/IO error surfaces when the config is loaded rather than being hidden. Also drop the absent-yaml negative cache so a workspace yaml created mid-process (e.g. by `aube add`) isn't shadowed by a cached default. Co-authored-by: Cursor <cursoragent@cursor.com>
…lockfile Under sharedWorkspaceLockfile=false a workspace root that is itself a package carries its own per-project lockfile, so the warm-path check only compared the root hash and skipped member lockfiles entirely. Decouple the member-lockfile staleness check from that branch so member add/remove/edit still busts the warm path. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
… a package Co-authored-by: Cursor <cursoragent@cursor.com>
workspace_yaml_kind warmed the typed config cache while classifying each ancestor during root discovery. A command that writes a workspace yaml mid-process (patch-commit, approve-builds, catalog edits) and then re-reads via WorkspaceConfig::load got the pre-write config back: patch- commit recorded a patch but the in-process reinstall reused a cached config without patchedDependencies and silently skipped applying it. Classify without warming the cache; WorkspaceConfig::load still memoizes for its own callers. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
Fixes several issues hitting workspace members under
sharedWorkspaceLockfile=false(per-project lockfiles), whereaube installfrom inside a member behaved incorrectly.pnpm-workspace.yaml: a member carrying a settings-only workspace yaml (nopackages:) is now treated as member config, not a standalone workspace, socd member && aube installresolves to the parent root and itsworkspace:siblings stay linked.node_moduleswas deleted is correctly re-linked instead of reporting "Already up to date".packageExtensionson git/tarball/dir deps: extensions were only applied on the registry resolution path. They're now applied to non-registry packages too, so an extension injecting a sibling dependency (e.g. a LoopBack connector a git-sourced packagerequire()s at runtime) is resolved and linked. Thename@*selector match is also hardened for non-semver (git) versions.Scoping a member install (no code change — pnpm parity)
A plain
aube installfrom anywhere in a workspace installs all importers, becauserecursiveInstalldefaults totrue(matching pnpm). To install just one service and the workspace siblings it depends on, use a filter with the trailing...:This installs
<service>+ its workspace deps (and their transitive deps) while leaving unrelated members untouched (nonode_modules, no lockfile written). Without..., only the named member is installed (sibling symlinked but its own deps not populated) — exactly pnpm's documentedfoovsfoo...semantics.Test plan
test/shared_workspace_lockfile_warm_path.bats— warm-path / member relink casestest/git_deps.bats—packageExtensionsinjected into a git packagetest/filter.bats—--filter <member>...scoping undersharedWorkspaceLockfile=falsecargo test(aube-resolver unit tests forpackageExtensionsflat-map + wildcard)cargo build/cargo clippy/cargo fmt --checkUpgrade note
Run
aube install --forceonce at the monorepo root to refresh lockfiles with thepackageExtensions-injected dependencies; afterwards plainaube install(and--filter '<service>...') stay on the warm path.Summary by CodeRabbit
New Features
Improvements
pnpm-workspace.yaml, and it stops correctly on invalid YAML.sharedWorkspaceLockfile=falsenow correctly merges per-member lockfile graphs so installs don’t skip required work.Tests