v0.3.5 bundle review: fix three cross-cutting issues before tag

The post-bundle holistic review caught three integration issues each
commit's individual review didn't surface:

1. **#85 wipe-confirm unreachable when calling R differs from pin
   (sync.rs).** The #70 calling-R bail fired before #85's interactive
   prompt, so users in the most common pinned-R-mismatch scenario got
   the opaque "Refusing to install" error instead of the wipe-confirm
   #85 was meant to deliver. Restructure the two guards into a single
   decision tree:
   - wipe needed + calling mismatched → combined warn + confirm; on
     accept, still bail with the calling-R explanation (project
     library left intact, accepted wipe runs on next sync from the
     matching R)
   - wipe needed only → existing #85 confirm path
   - calling mismatched only (no wipe) → existing #70 bail
   The user always sees ONE clear story instead of two competing
   guards.

2. **`uvr scan` missed roxygen2 @import / @importFrom (scan.rs).**
   Package source trees often declare deps exclusively via these
   tags, never calling library() in their own .R files. Without
   this regex, `uvr scan` reported "no references found" for those
   packages — actively misleading. Add a third regex; new test
   covers @importFrom dplyr filter mutate (extracts dplyr, NOT
   filter/mutate) and @import ggplot2.

3. **`.Rprofile` r-version warn collided with library messages
   (init.rs).** When .r-version != active R, the snippet emitted
   both "R 4.5 active but pin is 4.6" and "Run uvr::sync() to
   install" — users would chase the second (running sync builds
   packages for the wrong R, making the situation worse). Now the
   library-status messages are gated on version_ok; mismatched
   sessions see only the version warning.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: nbafrank

crates/uvr/src/commands/init.rs

@@ -135,6 +135,14 @@ local({
   # that warning and could be running scripts under the wrong R for
   # ages. Surface it at session startup so the mismatch is visible
   # immediately.
+  #
+  # Post-bundle review: when the version is mismatched, suppress the
+  # library-status messages below — otherwise the user sees both
+  # "R 4.5 active but pin is 4.6" and "Run uvr::sync() to install"
+  # and chases the wrong fix (running sync would build packages for
+  # the wrong R). Show the version warning alone; library messages
+  # become useful again once they restart R against the pin.
+  version_ok <- TRUE
   if (file.exists(rver_file)) {
     pinned <- tryCatch(trimws(readLines(rver_file, warn = FALSE)[1]),
                        error = function(e) "")
@@ -145,34 +153,37 @@ local({
       if (pinned_minor != active_minor) {
         message("uvr: R ", active, " active but .r-version pins ", pinned,
                 ". Restart R against the pinned version, or run uvr::r_pin() to change the pin.")
+        version_ok <- FALSE
       }
     }
   }
-  if (dir.exists(lib)) {
-    # #17: `.libPaths(lib)` with a single new path causes R to drop the
-    # user's site library (e.g. ~/R/x86_64-pc-linux-gnu-library/4.4) —
-    # only the new path and the system library survive. Prepending via
-    # `unique(c(lib, .libPaths()))` keeps the project lib first (so it
-    # wins resolution) while preserving anything the user already had.
-    .libPaths(unique(c(lib, .libPaths())))
-    n_locked <- count_locked(lock)
-    installed <- list.dirs(lib, recursive = FALSE, full.names = FALSE)
-    n_installed <- length(setdiff(installed, "uvr"))
-    if (n_locked > 0 && n_installed < n_locked) {
-      message("uvr: ", n_locked - n_installed, " of ", n_locked,
-              " package(s) not installed. Run uvr::sync() to install.")
-    } else if (n_locked > 0) {
-      message("uvr: library linked (", n_installed, " packages)")
+  if (version_ok) {
+    if (dir.exists(lib)) {
+      # #17: `.libPaths(lib)` with a single new path causes R to drop the
+      # user's site library (e.g. ~/R/x86_64-pc-linux-gnu-library/4.4) —
+      # only the new path and the system library survive. Prepending via
+      # `unique(c(lib, .libPaths()))` keeps the project lib first (so it
+      # wins resolution) while preserving anything the user already had.
+      .libPaths(unique(c(lib, .libPaths())))
+      n_locked <- count_locked(lock)
+      installed <- list.dirs(lib, recursive = FALSE, full.names = FALSE)
+      n_installed <- length(setdiff(installed, "uvr"))
+      if (n_locked > 0 && n_installed < n_locked) {
+        message("uvr: ", n_locked - n_installed, " of ", n_locked,
+                " package(s) not installed. Run uvr::sync() to install.")
+      } else if (n_locked > 0) {
+        message("uvr: library linked (", n_installed, " packages)")
+      } else if (file.exists(lock)) {
+        message("uvr: library active, but uvr.lock is empty. Run uvr::lock() to populate it.")
+      } else {
+        message("uvr: library active, but no uvr.lock found. Run uvr::lock() to create one.")
+      }
     } else if (file.exists(lock)) {
-      message("uvr: library active, but uvr.lock is empty. Run uvr::lock() to populate it.")
-    } else {
-      message("uvr: library active, but no uvr.lock found. Run uvr::lock() to create one.")
+      # #59: .uvr/library/ doesn't exist yet but the lockfile does — fresh
+      # checkout, never synced. Tell the user.
+      n_locked <- count_locked(lock)
+      message("uvr: 0 of ", n_locked, " package(s) installed. Run uvr::sync() to install.")
     }
-  } else if (file.exists(lock)) {
-    # #59: .uvr/library/ doesn't exist yet but the lockfile does — fresh
-    # checkout, never synced. Tell the user.
-    n_locked <- count_locked(lock)
-    message("uvr: 0 of ", n_locked, " package(s) installed. Run uvr::sync() to install.")
   }
 })
 # <<< uvr <<<

crates/uvr/src/commands/scan.rs

@@ -151,7 +151,7 @@ fn has_scannable_extension(path: &Path) -> bool {
     )
 }
 
-/// Compiled regexes for the four R-package name patterns.
+/// Compiled regexes for the R-package name patterns we recognise.
 ///
 /// Patterns are intentionally conservative — we want few false positives
 /// even at the cost of a few false negatives. Comment lines that contain
@@ -161,6 +161,7 @@ fn has_scannable_extension(path: &Path) -> bool {
 struct PackageDetector {
     library_or_require: Regex,
     namespace_op: Regex,
+    roxygen_import: Regex,
 }
 
 impl PackageDetector {
@@ -179,9 +180,18 @@ impl PackageDetector {
         let namespace_op =
             Regex::new(r"\b([A-Za-z][A-Za-z0-9._]*):{2,3}[A-Za-z._]").expect(":: regex compiles");
 
+        // roxygen2 `#' @import pkg` and `#' @importFrom pkg fn1 fn2`. Real
+        // package source trees often declare deps exclusively via these
+        // tags — without this, `uvr scan` returns "no references found"
+        // for any package that doesn't `library()` its own deps (post-
+        // bundle review).
+        let roxygen_import = Regex::new(r"#'\s*@import(?:From)?\s+([A-Za-z][A-Za-z0-9._]*)")
+            .expect("roxygen import regex compiles");
+
         Self {
             library_or_require,
             namespace_op,
+            roxygen_import,
         }
     }
 
@@ -197,6 +207,11 @@ impl PackageDetector {
                 found.insert(name.as_str().to_string());
             }
         }
+        for cap in self.roxygen_import.captures_iter(content) {
+            if let Some(name) = cap.get(1) {
+                found.insert(name.as_str().to_string());
+            }
+        }
         found
     }
 }
@@ -250,6 +265,31 @@ mixed <- dplyr::filter(df) |> tidyr::pivot_longer()
         assert!(found.is_empty(), "got {found:?}");
     }
 
+    #[test]
+    fn extract_roxygen_imports() {
+        // Package source trees often declare deps purely via roxygen2
+        // tags, never calling library() in their own .R files. Without
+        // this regex, `uvr scan` reports nothing for those packages.
+        let detector = PackageDetector::new();
+        let src = "\
+#' @importFrom dplyr filter mutate
+#' @importFrom rlang .data
+#' @import ggplot2
+#' @importFrom stats predict
+my_fn <- function() NULL
+";
+        let found = detector.extract(src);
+        assert!(found.contains("dplyr"), "got {found:?}");
+        assert!(found.contains("rlang"), "got {found:?}");
+        assert!(found.contains("ggplot2"), "got {found:?}");
+        assert!(found.contains("stats"), "got {found:?}");
+        // The function names after `@importFrom <pkg>` must NOT be
+        // captured as packages.
+        assert!(!found.contains("filter"), "got {found:?}");
+        assert!(!found.contains("mutate"), "got {found:?}");
+        assert!(!found.contains("predict"), "got {found:?}");
+    }
+
     #[test]
     fn extension_matching() {
         assert!(has_scannable_extension(Path::new("script.R")));

crates/uvr/src/commands/sync.rs

@@ -141,40 +141,56 @@ pub async fn install_from_lockfile(
         let current_minor = r_minor(current_r);
         let locked_minor = r_minor(&lockfile.r.version);
         let sentinel_minor = read_library_r_sentinel(&library);
-
-        // #70 guard — must fire BEFORE the wipe check, not inside it. A library
-        // already at `current_minor` (sentinel matches) skips the wipe but still
-        // installs new packages under the resolved R. From a calling R session
-        // on a different minor, those packages are unloadable in the live
-        // session. Bail unconditionally when the calling R differs from the
-        // R uvr would install for. Terminal-invoked uvr (R_HOME unset) returns
-        // None from calling_r_minor() and proceeds normally.
-        if let Some(calling_minor) = calling_r_minor() {
-            if calling_minor != current_minor {
-                anyhow::bail!(
-                    "Refusing to install: uvr is running inside R {calling} but the project pin/lockfile \
-                     resolves to R {target}. Packages built for R {target} would not load in this {calling} \
-                     session. Restart R against {target} (e.g. point your IDE at \
-                     ~/.uvr/r-versions/{target_full}/bin/R), or update the pin to match this session, \
-                     then re-run `uvr sync`.",
-                    calling = calling_minor,
-                    target = current_minor,
-                    target_full = current_r,
-                );
-            }
-        }
+        let calling_minor_opt = calling_r_minor();
 
         let lockfile_mismatch =
             looks_like_version(&lockfile.r.version) && current_minor != locked_minor;
         let sentinel_mismatch = sentinel_minor.as_ref().is_some_and(|m| m != &current_minor);
-
-        if lockfile_mismatch || sentinel_mismatch {
+        let wipe_needed = lockfile_mismatch || sentinel_mismatch;
+        let calling_mismatch = calling_minor_opt
+            .as_ref()
+            .is_some_and(|c| c != &current_minor);
+
+        // Two destructive risks to surface:
+        //   - sentinel/lockfile mismatch → about to wipe the project library
+        //   - calling R minor != install target → rebuilt library would not
+        //     load in the calling R session (#70).
+        // Pre-#85, the calling-R bail fired before the wipe-confirm prompt,
+        // making the prompt unreachable in the most common pinned-R-mismatch
+        // case (post-bundle review). Combine both signals into a single
+        // message + confirm path so the user sees one clear story.
+        if wipe_needed && calling_mismatch {
+            let from = sentinel_minor.clone().unwrap_or(locked_minor.clone());
+            let calling = calling_minor_opt.as_deref().unwrap_or("?");
+            ui::warn(format!(
+                "R version changed ({} {} {}) — about to wipe project library and reinstall, \
+                 but uvr is running inside R {calling} so the rebuilt library would NOT load in this session",
+                palette::dim(&from),
+                palette::dim(ui::glyph::arrow()),
+                palette::info(&current_minor),
+            ));
+            if !confirm_library_wipe(&library)? {
+                anyhow::bail!(
+                    "Aborted: not wiping project library. Restart R against the install target \
+                     (e.g. point your IDE at ~/.uvr/r-versions/{current_r}/bin/R) and re-run \
+                     `uvr sync`, or update the pin so it matches this {calling} session."
+                );
+            }
+            // User explicitly accepted — proceed with wipe + reinstall, but
+            // bail with the calling-R explanation before installing into a
+            // mismatched session (the rebuilt library still won't load here).
+            anyhow::bail!(
+                "Refusing to install: uvr is running inside R {calling} but the lockfile resolves \
+                 to R {target}. Restart R against {target} (e.g. point your IDE at \
+                 ~/.uvr/r-versions/{target_full}/bin/R), or update the pin to match this session, \
+                 then re-run `uvr sync`. (Project library left intact — accepted wipe will run \
+                 on the next sync from the matching R.)",
+                calling = calling,
+                target = current_minor,
+                target_full = current_r,
+            );
+        } else if wipe_needed {
             let from = sentinel_minor.unwrap_or(locked_minor);
-            // #85: clarify it's the *project* library being wiped (not the
-            // global cache); confirm before destructive actions when stdin
-            // is a TTY so a misdetected R version doesn't silently nuke
-            // hand-built packages (B-Nilson's case where Matrix and s2 had
-            // been custom-built per #52).
             ui::warn(format!(
                 "R version changed ({} {} {}) — about to wipe project library and reinstall",
                 palette::dim(&from),
@@ -193,6 +209,22 @@ pub async fn install_from_lockfile(
                 std::fs::remove_dir_all(&library).context("Failed to wipe project library")?;
             }
             std::fs::create_dir_all(&library).context("Failed to recreate library directory")?;
+        } else if calling_mismatch {
+            // #70 guard — pure-bail path when no wipe is needed (library is
+            // empty or sentinel matches current_minor). The bail is correct:
+            // installing new packages under R `current` would still produce
+            // artefacts unloadable in the calling `calling` session.
+            let calling = calling_minor_opt.as_deref().unwrap_or("?");
+            anyhow::bail!(
+                "Refusing to install: uvr is running inside R {calling} but the project pin/lockfile \
+                 resolves to R {target}. Packages built for R {target} would not load in this {calling} \
+                 session. Restart R against {target} (e.g. point your IDE at \
+                 ~/.uvr/r-versions/{target_full}/bin/R), or update the pin to match this session, \
+                 then re-run `uvr sync`.",
+                calling = calling,
+                target = current_minor,
+                target_full = current_r,
+            );
         }
     }