Skip to content

[repo-assist] Fix restore cache to detect missing .paket/load scripts - #4363

Merged
dsyme merged 2 commits into
masterfrom
repo-assist/fix-issue-3238-load-script-cache-eabd62044299d330
Aug 27, 2026
Merged

dsyme merged 2 commits into
masterfrom
repo-assist/fix-issue-3238-load-script-cache-eabd62044299d330

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

AI disclosure

This PR was created automatically by Repo Assist, an AI assistant helping maintain this repository.

Closes #3238

Root cause

RestoreProcess.fs's early-exit / restore-cache logic (readCache, canEarlyExit) only compares a SHA256 hash of paket.lock against a cached hash. It has no awareness of whether the .paket/load/ folder (generated when a group sets generate_load_scripts: true) still exists on disk. If a user deletes .paket/load/ without changing paket.lock, the cache reports "up to date" and paket restore becomes a silent no-op, never regenerating the load scripts.

Fix

Added a loadScriptsMissing check: if any lock-file group requires load scripts (GenerateLoadScripts = Some true) and the .paket/load folder is missing, the restore cache can no longer report an early exit, forcing CreateScriptsForGroups to run and regenerate the scripts. When no group requires load scripts, or the folder still exists, behaviour is unchanged (same hash-based caching as before), so this should not affect performance for the common case.

Trade-offs

  • Adds one Directory.Exists check per restore when canCacheRestore is true and at least one group has load scripts enabled - negligible cost compared to a full restore.
  • Does not detect partial deletion of individual script files (only the outer folder), which keeps the check simple; a more granular per-script check could be a future refinement if needed.
  • No new tests were added in this PR: the affected logic lives in a private, filesystem/lock-file-hash-driven code path inside Restore, and properly exercising it needs a full integration-test scenario (lock file + on-disk load folder deletion) rather than a unit test. Flagging this as a good follow-up for a maintainer or a future paket.load-focused integration test.

Test Status

  • dotnet restore src/Paket.Core/Paket.Core.fsproj -p:PaketDisableGlobalRestore=true - succeeded
  • dotnet build src/Paket.Core/Paket.Core.fsproj -f netstandard2.0 --no-restore -p:PaketDisableGlobalRestore=true - succeeded, 0 errors
  • dotnet test tests/Paket.Tests/Paket.Tests.fsproj -f net9 --no-restore -p:PaketDisableGlobalRestore=true - 1238 passed, 16 skipped, 6 failed. All 6 failures are pre-existing/environment-specific (proxy env-var tests in UtilsSpecs, a net461-only assembly-metadata test, and an XML-generation test) and reproduce identically on master without this change - confirmed no new regressions.

🤖 This pull request was created by Repo Assist, an automated AI assistant. A human maintainer should review before merging.

Generated by 🌈 Repo Assist, see workflow run. Learn more.
Comment /repo-assist to run again

Add this agentic workflow to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@ae8d551f07c7ed7619f8c58c7bb4c3ac89395d38

When a group has generate_load_scripts enabled but a user deletes the
.paket/load folder without touching paket.lock, the restore hash cache
previously reported 'up to date' and skipped restore entirely, so load
scripts were never regenerated. This adds a check for the presence of
the .paket/load folder (when required) to the early-exit condition.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dsyme
dsyme marked this pull request as ready for review August 26, 2026 22:15
@dsyme

dsyme commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

/repo-assist add an integration test for this?

@github-actions

github-actions Bot commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor Author

✓ Repo Assist completed successfully, see workflow run.

Generated by 🌈 Repo Assist, see workflow run. Learn more.

…3238

The earlier fix added a check to prevent the restore hash cache from
early-exiting when the .paket/load folder was missing, but the inner
restore branch only called CreateScriptsForGroups when packages were
not already up to date. Since packages remained up to date, the load
scripts were still never regenerated - reproduced manually before this
change.

This adds a call to CreateScriptsForGroups in the 'packages already up
to date' branch when the load scripts folder is missing, and adds an
integration test (dependencies-file-flag scenario) that deletes
.paket/load after install and verifies paket restore regenerates it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor Author

🤖 This is an automated response from Repo Assist.

I found and fixed a real gap in the earlier commit: it stopped the restore-hash cache from early-exiting, but the "packages already up to date" branch never actually called script generation, so .paket/load still wasn't regenerated after deletion. I reproduced this manually, fixed it, and added an integration test (#3238 restore regenerates load scripts when .paket/load folder is missing) covering: install → delete .paket/load → paket restore → assert scripts come back. New commit pushed to this PR; scriptgen suite (12 tests) and restore unit tests pass.

Generated by 🌈 Repo Assist, see workflow run. Learn more.
Comment /repo-assist to run again

Add this agentic workflow to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@ae8d551f07c7ed7619f8c58c7bb4c3ac89395d38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restore does not always regenerate load scripts

1 participant