Skip to content

fix: refresh loaded linker lockfile state - #7

Merged
zkochan merged 1 commit into
mainfrom
fix/loaded-lockfile-refresh
Oct 5, 2026
Merged

zkochan merged 1 commit into
mainfrom
fix/loaded-lockfile-refresh

Conversation

@zkochan

@zkochan zkochan commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Summary

refresh-lockfile now removes the current installation lockfile used by the loaded linker and explicitly configured installation directories. This prevents pnpm install from reusing old dependency resolutions after the wanted lockfile is deleted. Other files in those directories are preserved.

Closes pnpm/tasks#75.

Validation

  • 35 Bats tests passed.
  • Shellcheck passed for the action scripts.
  • Two real-install regressions passed with a pnpm build supporting the loaded linker. Both isolated and loaded installs selected a newly published matching version after refreshing.

Written by an agent (Codex, GPT-6).

Summary by CodeRabbit

  • New Features
    • Lockfile refresh now clears the current installation lockfile and relevant installation state before updating dependencies, allowing available versions to be resolved again.
    • Refresh respects configured installation paths and preserves unrelated files in those directories.
  • Documentation
    • Clarified which files are removed during a refresh and added instructions for running the lockfile refresh tests with a compatible pnpm build.
  • Tests
    • Added coverage for refresh behavior across linker types and custom installation paths.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 440cf1dd-2ffd-49cf-bb86-c69e228b3276
📥 Commits

Reviewing files that changed from the base of the PR and between fcaa952 and 4f7d063.

📒 Files selected for processing (7)
  • README.md
  • action.yml
  • scripts/refresh-lockfile.mjs
  • scripts/update.sh
  • test/refresh-lockfile.test.mjs
  • test/stubs/pnpm
  • test/update.bats

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The refresh path now selects an installation state directory from pnpm configuration and removes its current lockfile before updating. Tests cover lockfile refresh with isolated and loaded linkers, configured paths, and newer matching package versions.

Changes

Installation lockfile refresh

Layer / File(s) Summary
Resolve and clear installation state
action.yml, README.md, scripts/refresh-lockfile.mjs, scripts/update.sh
The refresh script resolves the modules and state directories from pnpm configuration, then removes the selected state directory’s lock.yaml, the root pnpm-lock.yaml, and node_modules. The update script invokes the refresh script. The action description and README describe removal of the current installation lockfile.
Validate refresh behavior
test/refresh-lockfile.test.mjs, test/stubs/pnpm, test/update.bats, README.md
Tests verify refreshed resolutions with isolated and loaded linkers, removal of lockfiles from configured paths, and preservation of unrelated files. The README adds test-running instructions and specifies the required pnpm build.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 4f7d0

The change makes lockfile refresh also remove the current installation lockfile, and it is covered by isolated and loaded linker tests. No concrete merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4f7d0

The change narrowly removes the configured installation lockfile and retains the existing update workflow. No new credential access or arbitrary command execution was identified. Remaining uncertainty concerns externally configured installation directories, shared-runner recovery, and compatibility across pnpm versions.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The newly selected deletion targets a fixed lock.yaml filename without recursive removal. Its parent can be an absolute, home-relative or workspace-relative configured directory, with no workspace-confinement or ownership check. Effective reach is bounded by the runner process's filesystem permissions; recursive removal of root node_modules already existed.

Security Findings and Attack Paths

  • inferred — An actor controlling effective pnpm directory configuration could direct the new deletion toward another writable lock.yaml. The inspected default flow uses the fetched base branch, and installation already consumes pnpm configuration. The evidence does not establish lower-trust configuration control, additional privileges, or cross-tenant exposure, so this possibility is not retained as a verified PR vulnerability.

Trust Boundaries and Controls

  • observed — The documented workflow uses scheduled or manually dispatched updates, excludes forks, and serializes its update concurrency group. These are example deployment controls, not enforcement supplied by the composite action for every consumer or every shared installation directory.

Resilience and Maintainability Implications

  • inferred — Missing files are tolerated, making repeated deletion retry-friendly. The separate deletions are nevertheless nontransactional and have no helper-level synchronization or rollback. Partial mutation of an external installation directory is possible, but its effect on other consumers depends on directory sharing and pnpm recovery behavior not established here.

Hardening Proposals

  • proposed — Document exclusive ownership expectations for configured installation directories. Where persistent runners share installation state, use isolation or coordination covering all consumers rather than relying only on the example update-workflow concurrency group.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #75 requires refresh-lockfile to remove installation lock state for isolated and loaded linkers, respect configured installation paths, preserve unrelated files, and test fresh resolution with u…
Out of Scope Changes check ✅ Passed The action description and README document the refresh behavior. The script integration and test stub support the new lockfile handling. The added tests verify issue #75 requirements. The reviewed cha…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: refreshing lockfile state for the loaded linker.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

A rabbit checks the lockfile door,
And clears the stale state from the floor.
New versions hop into the view,
While saved files stay safe and true.
The linker paths are checked with care,
Then fresh installs spring everywhere.

Comment @coderabbitai help to get the list of available commands.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@qodo-code-review

qodo-code-review Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

PR Summary by Qodo

Fix lockfile refresh for loaded and custom pnpm installations

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Remove the active installation lockfile so refreshes cannot reuse stale dependency resolutions.
• Preserve unrelated files in configured installation directories while retaining existing workspace
 cleanup.
• Add isolated and loaded linker regressions and document the refresh behavior.
Diagram

graph TD
  A["Action input"] --> B["Update script"] --> C["Refresh helper"] --> D["pnpm config"] --> E["Installation lockfile"]
  C --> F["Workspace lockfile"]
  C --> G["node_modules"]
  B --> H["pnpm install"]
Loading
High-Level Assessment

The scoped cleanup is appropriate: reading pnpm's effective JSON configuration handles linker and directory settings, while deleting only the active installation lockfile preserves other installation files. Removing whole configured directories would be unnecessarily destructive.

Files changed (7) +167 / -5

Bug fix (2) +25 / -3
refresh-lockfile.mjsRemove the active installation lockfile +24/-0

Remove the active installation lockfile

• Reads effective pnpm configuration to locate the active lock.yaml for loaded or isolated installations, accounting for configured directories. Removes it alongside pnpm-lock.yaml and node_modules while leaving other files in installation directories intact.

scripts/refresh-lockfile.mjs

update.shRun configuration-aware cleanup when refresh is enabled +1/-3

Run configuration-aware cleanup when refresh is enabled

• Replaces the inline deletion command with the new Node helper before the install or update step.

scripts/update.sh

Tests (3) +133 / -0
refresh-lockfile.test.mjsVerify fresh resolution with real isolated and loaded installs +89/-0

Verify fresh resolution with real isolated and loaded installs

• Adds local-registry integration tests that publish a newer matching fixture version after the initial install. Each linker must select the newer version after running the refresh path.

test/refresh-lockfile.test.mjs

pnpmStub effective pnpm configuration +9/-0

Stub effective pnpm configuration

• Handles pnpm config list with configurable JSON output so Bats tests can exercise linker and directory settings.

test/stubs/pnpm

update.batsCover scoped cleanup for loaded and custom installations +35/-0

Cover scoped cleanup for loaded and custom installations

• Adds cases for the loaded linker's active lockfile, a configured modules directory, and an isolated linker's custom virtual store. Assertions also verify that unrelated installation files and inactive lockfiles remain.

test/update.bats

Documentation (2) +9 / -2
README.mdDocument installation-state refresh and regression test command +7/-1

Document installation-state refresh and regression test command

• Explains that refresh removes the active installation lockfile without deleting other files in its directory. Adds the real-pnpm regression test command and clarifies that CI runs shellcheck and Bats, not the loaded-linker tests.

README.md

action.ymlClarify the refresh-lockfile input +2/-1

Clarify the refresh-lockfile input

• The input description now includes the current installation lockfile among the state removed before dependency resolution.

action.yml

@zkochan
zkochan merged commit 2457663 into main Oct 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support loaded-linker installation state in pnpm/update lockfile refresh

1 participant