fix(pak): verify installed version matches pak's resolution before reporting OK

R/pak.R

@@ -443,6 +443,32 @@ isGH <- function(pkgs) {
   grepl("^[[:alpha:]]+/.+", pkgs)
 }
 
+# Returns TRUE iff the constraint (`versionSpec` / `inequality`) is satisfied,
+# either by what's on disk (`installedVer`) or by pak's globally-resolved
+# version (`pakResolvedVer`) -- and in the latter case only when pak's
+# resolution matches what's actually on disk.
+#
+# The disk-match guard is the regression fix for the binary-lag scenario:
+# pak's `pkg_deps` resolver may pick the source version (e.g. reproducible
+# 3.1.0) while `pak::pak()` chooses to "keep" the older binary (3.0.0) on
+# platforms whose CRAN binary hasn't caught up. Treating `pakResolvedVer` as
+# authoritative without checking disk reality marks the unsatisfied install
+# as "OK" and the user gets `Installed 1 packages` followed by a still-old
+# `packageVersion(pkg)`.
+pakConstraintSatisfied <- function(installedVer, versionSpec, inequality,
+                                   pakResolvedVer = NA_character_) {
+  satisfies <- isTRUE(compareVersion2(installedVer,
+                                      versionSpec = versionSpec,
+                                      inequality  = inequality))
+  if (!satisfies && !is.na(pakResolvedVer) && nzchar(pakResolvedVer) &&
+      identical(pakResolvedVer, installedVer)) {
+    satisfies <- isTRUE(compareVersion2(pakResolvedVer,
+                                        versionSpec = versionSpec,
+                                        inequality  = inequality))
+  }
+  satisfies
+}
+
 # For each plain CRAN ref in `pkgsForPak` that names a package currently
 # installed in `libPaths`, rewrite it to `pkg@<installedVersion>`. Leaves
 # GitHub refs, pre-pinned `pkg@X` refs, packages with an explicit user
@@ -3327,11 +3353,9 @@ pakInstallFiltered <- function(pkgDT, libPaths, repos, standAlone, verbose,
           pakVerMap <- get0("pakResolvedVersionMap", envir = pakEnv(), inherits = FALSE)
           if (!is.null(pakVerMap)) {
             cand <- pakVerMap[pkg]
-            if (!is.na(cand) && nzchar(cand)) {
-              pakRes <- unname(cand)
-              satisfies <- isTRUE(compareVersion2(pakRes, versionSpec = vSpec, inequality = ineq))
-            }
+            if (!is.na(cand) && nzchar(cand)) pakRes <- unname(cand)
           }
+          satisfies <- pakConstraintSatisfied(installedVer, vSpec, ineq, pakRes)
         }
         if (!isTRUE(satisfies)) {
           # We are inside `if (NROW(nowRow))`, i.e. pak HAS something installed
@@ -3350,7 +3374,18 @@ pakInstallFiltered <- function(pkgDT, libPaths, repos, standAlone, verbose,
           # on disk) and warrants the "please change required version" guidance
           # even when preVer == installedVer (e.g. on a re-Require() call).
           pakChoseInstalled <- !is.na(pakRes) && identical(pakRes, installedVer)
-          if (versionChanged || firstTimeInsufficient || pakChoseInstalled)
+          # pak resolved to a newer version than what's actually on disk
+          # (typical Mac/Win case: source > binary). Warn the user that the
+          # install was effectively a no-op even though pak reported success.
+          pakResolvedNewer <- !is.na(pakRes) && !identical(pakRes, installedVer) &&
+                              isTRUE(compareVersion2(pakRes, versionSpec = vSpec, inequality = ineq))
+          if (pakResolvedNewer) {
+            warning(.txtCouldNotBeInstalled, ": ", pkg, " ", ineq, " ", vSpec,
+                    "; pak resolved ", pakRes, " but only ", installedVer,
+                    " is available as a binary on this platform -- ",
+                    "install from source or wait for the binary to be built.",
+                    call. = FALSE)
+          } else if (versionChanged || firstTimeInsufficient || pakChoseInstalled)
             warning(msgPleaseChangeRqdVersion(pkg, ineq = ">=", newVersion = installedVer), call. = FALSE)
           # Always add to warnedDropped: either we already warned above (versionChanged),
           # or pak ran and chose not to update this package, meaning Require's over-strict

tests/testthat/test-16installFailureMetadata_testthat.R

@@ -694,3 +694,71 @@ test_that("identify-and-defer recovers from PSPclean-style cascade", {
     expect_true(all(nzchar(failures$reason_brief)))
   }
 })
+
+# ---------------------------------------------------------------------------
+# Regression: binary-lag silent "OK"
+#
+# Field case (Mac, Require 2.0.0):
+#   > Require::Install("reproducible (>= 3.1.0)")
+#   Installed 1 packages in 6.4 secs
+#   > packageVersion("reproducible")
+#   [1] '3.0.0'
+#
+# CRAN had reproducible 3.1.0 in source but only 3.0.0 as a Mac binary, so
+# pak's `pkg_deps` resolver pinned 3.1.0 in `pakResolvedVersionMap` while
+# `pak::pak()` chose to "keep" the 3.0.0 binary. The post-install
+# reconciliation in pakInstallFiltered fell back to `pakResolvedVer` without
+# checking that it matched what was actually on disk, so the install was
+# reported as successful. The fix: only trust pak's resolved version when
+# it matches the version actually present.
+# ---------------------------------------------------------------------------
+test_that("pakConstraintSatisfied is FALSE when pak resolved newer than installed", {
+  # Binary-lag scenario: disk has 3.0.0, pak resolved to 3.1.0.
+  # Must be FALSE -- the install did not satisfy >= 3.1.0.
+  expect_false(Require:::pakConstraintSatisfied(
+    installedVer   = "3.0.0",
+    versionSpec    = "3.1.0",
+    inequality     = ">=",
+    pakResolvedVer = "3.1.0"
+  ))
+})
+
+test_that("pakConstraintSatisfied is TRUE when installed version meets constraint", {
+  expect_true(Require:::pakConstraintSatisfied(
+    installedVer   = "3.1.0",
+    versionSpec    = "3.1.0",
+    inequality     = ">=",
+    pakResolvedVer = "3.1.0"
+  ))
+  expect_true(Require:::pakConstraintSatisfied(
+    installedVer   = "3.2.0",
+    versionSpec    = "3.1.0",
+    inequality     = ">=",
+    pakResolvedVer = NA_character_
+  ))
+})
+
+test_that("pakConstraintSatisfied is FALSE when neither installed nor pakRes meets constraint", {
+  # pak chose to install installedVer (matches what's on disk) but neither
+  # satisfies the user constraint -- the install is a real failure.
+  expect_false(Require:::pakConstraintSatisfied(
+    installedVer   = "3.0.0",
+    versionSpec    = "3.1.0",
+    inequality     = ">=",
+    pakResolvedVer = "3.0.0"
+  ))
+})
+
+test_that("pakConstraintSatisfied tolerates missing / blank pakResolvedVer", {
+  # When pakResolvedVer is unavailable, fall back solely to installedVer.
+  expect_false(Require:::pakConstraintSatisfied(
+    installedVer = "3.0.0", versionSpec = "3.1.0", inequality = ">="))
+  expect_false(Require:::pakConstraintSatisfied(
+    installedVer = "3.0.0", versionSpec = "3.1.0", inequality = ">=",
+    pakResolvedVer = NA_character_))
+  expect_false(Require:::pakConstraintSatisfied(
+    installedVer = "3.0.0", versionSpec = "3.1.0", inequality = ">=",
+    pakResolvedVer = ""))
+  expect_true(Require:::pakConstraintSatisfied(
+    installedVer = "3.1.0", versionSpec = "3.1.0", inequality = ">="))
+})