Repository navigation
fix: respect --home precedence when uninstalling models - #164
MuhammadAashirAslam wants to merge 1 commit into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthrough
ChangesModel directory lookup
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Uninstall follows the requested home precedence and avoids deleting a model store from another configured base. No actionable merge-blocking risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change narrows model-store selection to the requested home and preserves the confirmation gate. No introduced security vulnerability was established. Low residual uncertainty remains because deletion behavior during concurrent filesystem changes and interrupted execution could not be fully compared with the previous version. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
This change looks good - can we test this @Siddhesh2377 ? |
|
Superseded: the --home precedence fix is cherry-picked into #168. Closing as redundant — review continues there. |
Fix: respect --home when selecting models for uninstall
Summary
wally uninstallcould select the model store fromRUNANYWHERE_HOMEeven whenan explicit
--homepointed elsewhere. Confirming uninstall could thereforedelete models from the wrong store.
This completes the override handling added in
PR #133.
Changes
--hometakes priority overRUNANYWHERE_HOME.Only use kit/platform fallback discovery when neither override is set.
RunAnywhere/ModelsandModels.after each test, including on failure.
No SDK, dependency pin, confirmation, or deletion-mechanism changes.
Validation
passed on Windows with the pinned Rust 1.93.1 toolchain.
Windows denied symlink creation (OS error 1314) in
cmd_modelsandcmd_rm.cargo clippy --release --locked --all-targets -- -D warningsis blocked bythree pre-existing Windows dead-code warnings in
src/harness/path_reload.rs.Targeted test command (after CMake configuration and build-environment setup):
cargo test --release --locked --lib commands::cmd_maintenance::testsLocal test linking used a temporary import library under ignored
build/for theinstalled Git OpenSSL DLL. Without it, the existing Windows kit fails to link
OPENSSL_thread_stop. The original linker settings were restored after testing;the workaround is not part of the PR.
The agent-sync check reports the checkout's existing
CLAUDE.mdregular-fileissue; it requires a symlink to
AGENTS.md. No agent files were changed.Full CTest, smoke/e2e, and macOS verification have not been run for this change.
Summary by cubic
Fixes
wally uninstallselecting the model store fromRUNANYWHERE_HOMEeven when an explicit--homepointed elsewhere.--homenow takes precedence, and any override confines the search to that base, so confirming an uninstall can no longer delete models from the wrong store.RunAnywhere/ModelsandModelslayouts.--homenorRUNANYWHERE_HOMEis set.Written for commit 544d515. Summary will update on new commits.
Summary by CodeRabbit
--home, thenRUNANYWHERE_HOME, and uses the platform default only when neither is set.--homeis supplied, missing model stores are reported within that location without falling back to other directories.