fix(install): track actual hoisted direct entries - #1295
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change records visible ancestor paths for hoisted dependencies in ChangesHoisted dependency visibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change records the actual locations of hoisted direct dependencies while preserving fallback paths, with targeted regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Greptile SummaryThe PR corrects install freshness state for hoisted workspaces by recording the nearest ancestor-visible placement of each direct dependency while retaining importer-local fallbacks. It adds unit and BATS coverage for repeated warm script checks when a workspace dependency is shared at the root. Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or repository-rule issues identified. The placement lookup is scoped by the dependency’s resolved dep path, follows Node’s nearest-ancestor resolution order, and preserves existing local fallback behavior for isolated or unavailable planner state. Important Files Changed
Reviews (1): Last reviewed commit: "fix(install): track actual hoisted direc..." | Re-trigger Greptile |
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
Summary
Root cause
The install state always recorded
<importer>/node_modules/<name>for every direct dependency. Workspace-wide hoisting intentionally leaves that path absent when the dependency is shared from an ancestornode_modules, so every freshness check treated a valid layout as stale and reinstalled the same tree.Validation
cargo test -p aube state::testscargo clippy --all-targets -- -D warningscargo fmt --checkmise run test:bats test/hoisted.bats --filter 'hoisted workspace root placements stay warm'AI-assisted — Tool: Codex; model: unavailable; version: unavailable.
Note
Medium Risk
Changes install freshness/layout verification on a hot path (
aube run); behavior is scoped to hoisted linker with placements, with isolated fallback unchanged.Overview
Fixes false install warm-path misses in hoisted workspaces where a member’s direct dependency is satisfied from the root
node_modulesand the member-local slot is intentionally empty.When persisting
direct_entriesinInstallLayoutState::from_graph, the code no longer always records<importer>/node_modules/<name>. For hoisted installs it usesHoistedPlacementsto store the first ancestor-visible path Node can resolve (e.g.node_modules/is-numberforpackages/app), walking importer ancestors until a recorded placement matches. Isolated mode and missing placement data still use the local importer path.Adds a unit test for hoisted workspace importers and a bats case that repeated
aube rundoes not trigger Auto-installing when deps live only at the workspace root.Reviewed by Cursor Bugbot for commit 44f16e5. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit