Repository navigation
feat(embed): add per-install storage overrides - #1239
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe embedding API now accepts host-specific cache, store, and virtual-store overrides. These overrides flow through settings resolution, embedded add operations, installation, trust-policy validation, packument fetching, storage access, and stored-path serialization. ChangesEmbedded installation overrides
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant EmbedAPI
participant SettingsContext
participant Install
participant Store
Host->>EmbedAPI: install_with_overrides(options, overrides)
EmbedAPI->>SettingsContext: scope overrides and append CLI settings
SettingsContext->>Install: resolve cache, store, and virtual-store paths
Install->>Store: open context-aware store
Install->>Install: fetch using resolved packument cache
Install-->>EmbedAPI: installation result
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryThe PR adds invocation-scoped native storage overrides for embedded install and add operations while preserving source-compatible APIs.
Confidence Score: 5/5The PR appears safe to merge. The previously reported warm-path and native-path failures are addressed, and no blocking failure remains. Important Files Changed
Reviews (4): Last reviewed commit: "fix(embed): preserve native storage path..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/aube/tests/embed.rs (1)
103-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise package storage and materialization.
Both tests can pass without a registry package entering the content-addressable store. The empty install has no package. The
workspace:*add resolves locally. Add a seeded cached registry dependency, then assert its project-local materialization and the absence of a global virtual-store entry.
crates/aube/tests/embed.rs#L103-L121: add a cached registry dependency topackage.jsonbeforeinstall_with_overrides.crates/aube/tests/embed.rs#L157-L176: add or replace the workspace dependency with a seeded cached registry dependency foradd_with_overrides.🤖 Prompt for 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. In `@crates/aube/tests/embed.rs` around lines 103 - 121, Add a seeded cached registry dependency to package.json before install_with_overrides at crates/aube/tests/embed.rs:103-121, then assert its project-local materialization alongside the existing store checks and retain the absence of the global virtual-store entry. At crates/aube/tests/embed.rs:157-176, add or replace the workspace:* dependency with the same seeded cached registry dependency before add_with_overrides, ensuring that test exercises registry package storage and project-local materialization.
🤖 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/src/commands/install/args.rs`:
- Around line 340-344: Update append_to to be fallible and propagate its error
to callers. When converting cache_dir and store_dir paths, use Path::to_str()
and return an error if either returns None; do not use to_string_lossy(),
ensuring invalid UTF-8 host paths are rejected rather than rewritten.
---
Nitpick comments:
In `@crates/aube/tests/embed.rs`:
- Around line 103-121: Add a seeded cached registry dependency to package.json
before install_with_overrides at crates/aube/tests/embed.rs:103-121, then assert
its project-local materialization alongside the existing store checks and retain
the absence of the global virtual-store entry. At
crates/aube/tests/embed.rs:157-176, add or replace the workspace:* dependency
with the same seeded cached registry dependency before add_with_overrides,
ensuring that test exercises registry package storage and project-local
materialization.
🪄 Autofix
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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 08582b29-ef76-4912-92b8-5028d2378e1e
📒 Files selected for processing (15)
crates/aube/src/commands/add/filtered.rscrates/aube/src/commands/add/manifest.rscrates/aube/src/commands/add/mod.rscrates/aube/src/commands/add/supply_chain.rscrates/aube/src/commands/install/args.rscrates/aube/src/commands/install/fetch.rscrates/aube/src/commands/install/gvs.rscrates/aube/src/commands/install/lifecycle.rscrates/aube/src/commands/install/mod.rscrates/aube/src/commands/install/settings.rscrates/aube/src/commands/install/startup.rscrates/aube/src/commands/mod.rscrates/aube/src/commands/settings_context.rscrates/aube/src/embed.rscrates/aube/tests/embed.rs
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 326ef62. Configure here.
Instruction counts
1 benchmark(s) above the 1% gate: 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.
|
jdx
added a commit
to jdx/mise
that referenced
this pull request
Aug 7, 2026
## Summary - update the embedded `aube` and `aube-registry` crates to v1.38.0 - refresh the aube workspace dependency graph in `Cargo.lock` ## Why aube v1.38.0 publishes the embedder storage overrides from aubepkg/aube#1239 and structured deprecation output from aubepkg/aube#1236. Keeping the dependency bump separate makes the follow-up mise integration changes easier to review. Upstream release: aubepkg/aube#1202 ## Validation - `cargo check` *AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes where embedded aube keeps packuments and file store (existing global aube data may be re-fetched); install behavior otherwise stays on the same per-tool layout and lockfile names. > > **Overview** > Bumps embedded **`aube`** / **`aube-registry`** to **1.38.0** (lockfile refresh, including **`usage-lib` 5.0** and transitive updates). > > Embedded **`npm:`** installs now call **`aube::embed::add_with_overrides`** with mise-owned **`$MISE_CACHE_DIR/npm/aube/cache`** and **`.../store`**, **`use_global_virtual_store: false`**, so regenerable aube state is prunable via **`mise cache prune npm`** instead of relying on aube’s global virtual store. Per-tool installs still land in the tool prefix (`node_modules`, lockfile). > > **`aube_host`** docs/tests now stress **config name compatibility** (lockfile, `aube.allowBuilds`) rather than claiming the host profile pins all on-disk layout. E2e and a unit test lock in the new cache/store paths. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4b9b830. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Improved embedded npm installation reliability by using consistent, invocation-scoped cache and storage locations. * Ensured installation state can be regenerated and remains isolated between invocations. * **Chores** * Updated embedded Aube dependencies to version 1.38. * Added regression coverage for npm installation storage behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Embedding hosts need to own the complete lifecycle of installed packages and regenerable metadata. Process environment and project configuration were not sufficient because they could redirect embedded installs back into shared aube namespaces or enable the global virtual store.
These explicit overrides have command-line precedence without mutating process-global state, allowing mise to isolate embedded npm installs and remove their storage during mise prune.
Validation
AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.
Note
Medium Risk
Changes core install/fetch/store resolution and embedder isolation paths; incorrect override wiring could mis-link or reuse the wrong cache/store, though behavior is covered by new embed and settings-hash tests.
Overview
Adds
EmbedderInstallOverridesandinstall_with_overrides/add_with_overridesso embedding hosts can pincacheDir,storeDir, andenableGlobalVirtualStorefor a single call without mutating process env or global defaults.cache_dir/store_dirare carried in a tokio task-local (nativePathBufs, including non-UTF-8 on Unix);use_global_virtual_storeis injected into the CLI settings bag at command precedence.Install, embedded add, supply-chain gates, manifest packument lookups, trust-policy validation caches, resolver packument dirs, GVS reset/detection, and the install warm-path settings hash now resolve storage through
with_settings_ctx_and_cliandopen_store_with_ctx/resolved_*_with_ctxso overrides stay consistent for the whole invocation.fetch_packages_with_roottakes an explicit packument cache path; an explicit store override disables the “already linked” fetch shortcut so a new store is not skipped.StoredFile.store_pathgains custom serde (UTF-8 string or platform-native bytes) so host-owned cache indices round-trip non-UTF-8 paths.Reviewed by Cursor Bugbot for commit e302759. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes