claude.yml restores renv from the BASE branch's lockfile: Restore renv runs before Checkout PR branch

[d-morrison]

Summary

In .github/workflows/claude.yml, Restore renv (line 487) runs before
Checkout PR branch (line 531). So on a PR, the renv library is restored from
the base branch's renv.lock, never the PR's.

Two consequences, and the second is the one that cost a day:

1. A PR that changes renv.lock is never validated against its own
   lockfile. The one workflow that calls renv::restore() tests the base
   branch's lockfile instead.
2. A PR that adds a dependency gets a library that lacks it. Claude then
   runs the repo's checks against a library that does not satisfy the
   lockfile in the working tree.

How it surfaced

ucdavis/bcs had two rotted renv.lock records
(bcs#602): a GitHub pin whose
upstream commit had been garbage-collected, and a Repository/CRAN record at
a development version CRAN never published. renv::restore() failed on a cold
cache.

Fixing them in bcs#603 and firing
this workflow on that PR reproduced the failure verbatim, with the old
version numbers:

✖ snapr 0.0.0.9000
✖ cards 0.8.0.9004

Those are the values from main, not from the PR. The run confirms it
directly --- event: issue_comment, head_branch: main,
head_sha: 448971f8 (the PR head is a different commit) --- and the job's step
list shows why:

#	Step	Result
13	Restore renv	failure
15	Capture PR head SHA before Claude	skipped
16	Checkout PR branch	skipped

The restore fails at step 13, so steps 15 and 16 never run --- but even on a
passing run the ordering holds and the lockfile read is the base branch's.

Why this is worth fixing rather than documenting

It makes a whole class of change unverifiable by the only workflow that could
verify it, and it does so silently: the run fails (or passes) on evidence
from a different tree, with nothing in the output saying which lockfile it
read. In bcs's case it also helps explain why the rot went unnoticed for so
long --- even a PR written to fix it could not demonstrate the fix.

Suggested fix

Move the PR-branch checkout before the R/renv setup block, so everything
downstream reads the PR's own tree. The steps in between (Install packages,
Set up pandoc/R/Quarto) do not depend on repository content, so the move
looks low-risk, but I have not tested it.

Two details worth handling in the same change:

• The renv cache key is derived from renv.lock, so reading the PR's
  lockfile changes the key. That is correct behaviour --- a different lockfile
  should not hit the base branch's cache --- but it means the first run after
  this fix on any renv PR will be a cold restore.
• Checkout PR branch currently carries a downstream fallback (lines
  1102-1105) for the case where it "failed or was skipped (e.g. setup-renv
  failed)". Moving the checkout earlier removes that particular dependency,
  since the checkout would no longer sit behind a step that can fail first.

Same ordering question applies to preview/action.yml and
quarto-publish/action.yml, which set the same RENV_CONFIG_INSTALL_REMOTES
env; I have not checked whether they have an equivalent PR-checkout step.

[d-morrison]

Resolving the hedge I closed the issue body with. I wrote that "the same
ordering question applies to preview/action.yml and quarto-publish/action.yml
... I have not checked". I have now, and neither is affected. The scope is
narrower than I implied, and the reason is the interesting part.

File	Checkout	renv restore	Affected?
.github/workflows/claude.yml	Checkout PR branch, line 531	line 487	yes --- restore is 44 lines earlier
preview/action.yml	Check out repository, line 46	line 106	no --- checkout is 60 lines earlier
quarto-publish/action.yml	none; the caller checks out	line 68	no --- inherits the caller's tree

preview/action.yml gets it right by ordering, and by using a plain
actions/checkout with no ref:, which on a pull_request event resolves to
the PR's own merge ref.

So this is not a general pattern across the repo. It is specific to
claude.yml, and specifically to the combination that makes claude.yml
unusual:

• it is triggered by issue_comment, which checks out the default branch
  regardless of which PR the comment is on, so the initial Checkout repository
  cannot be the PR's tree; and
• it therefore needs a second, manual Checkout PR branch step to reach
  the PR --- and that step sits after the R/renv block rather than before it.

That narrows the fix, too. Moving the PR checkout earlier is a change to one
workflow rather than a repo-wide reordering, so the blast radius is smaller
than the issue body suggested.

One thing I still have not tested: whether anything between lines 487 and 531
depends on the workspace holding the base branch specifically. I do not
believe so --- the steps in between install packages and set up
pandoc/R/Quarto --- but that is a reading, not a test.