Skip to content

fix: normalize Windows cache store paths - #22

Merged
zkochan merged 2 commits into
mainfrom
fix-windows-cache-path
Aug 7, 2026
Merged

fix: normalize Windows cache store paths#22
zkochan merged 2 commits into
mainfrom
fix-windows-cache-path

Conversation

@zkochan

@zkochan zkochan commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

  • strip the Windows extended-length prefix from pnpm store path before passing the path to @actions/cache
  • preserve extended UNC paths by converting them back to regular UNC paths
  • add a Windows end-to-end cache job that forces and verifies the affected store-path form
  • rebuild the committed action bundle

Why

On Windows, pnpm can return a store path such as \\?\D:\.pnpm-store\v11. The cache restore accepts that path, but the post-job save rejects the ? as a glob in the root segment:

Invalid pattern '\?\D:.pnpm-store\v11'. Root segment must not contain globs.

The consumer's tests all passed, but both Windows matrix jobs failed during Post Run pnpm/setup@v2:
https://github.com/pnpm/cmd-shim/actions/runs/31169376044/job/92837467051

Validation

  • pnpm exec tsc --noEmit
  • pnpm run build
  • workflow and action YAML parsing
  • local drive-path, UNC-path, and unchanged POSIX-path normalization cases

Summary by CodeRabbit

  • Bug Fixes

    • Improved Windows cache restoration by handling extended-length paths correctly.
    • Preserved compatibility with standard drive and network paths.
    • Improved reliability when configuring and locating the package store on Windows.
  • Tests

    • Added Windows coverage to verify extended-length package store paths are handled as expected.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@zkochan, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3309a21c-4020-4668-83e3-fd84fc53cbc2

📥 Commits

Reviewing files that changed from the base of the PR and between 20c4456 and bad5177.

📒 Files selected for processing (1)
  • .github/workflows/test.yaml
📝 Walkthrough

Walkthrough

The cache directory now normalizes Windows extended-length pnpm store paths. A Windows workflow configures a root-level store and verifies that pnpm reports the extended-length path format.

Changes

Windows cache path handling

Layer / File(s) Summary
Path normalization
src/cache-restore/run.ts
getCacheDirectory normalizes the pnpm store path. The exported removeWindowsExtendedPathPrefix helper handles drive paths, UNC paths, and ordinary paths.
Windows workflow validation
.github/workflows/test.yaml
The cache-windows job configures a root-level pnpm store, enables caching, and verifies the extended-length path prefix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • pnpm/setup#1: Introduces the getCacheDirectory helper that this change extends.

Poem

A rabbit checked the Windows store,
Where extended paths ran the floor.
Prefix trimmed, UNC made neat,
Cache paths now hop on repeat.
pnpm smiles: “The route is sweet!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: normalizing Windows cache store paths.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-windows-cache-path

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

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Normalize Windows pnpm store paths before caching

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Strip Windows extended-length (\\?\) prefixes from pnpm store path before caching.
• Convert extended UNC paths back to regular UNC paths for @actions/cache compatibility.
• Add a Windows workflow job that forces the problematic store-path form and asserts behavior.
Diagram

graph TD
  W["CI: cache-windows"] --> A["pnpm/setup action"] --> B(["pnpm CLI"]) --> C["raw store path"] --> D["strip \\?\\ / UNC"] --> E[["@actions/cache"]] --> F[("GitHub Cache")]
  subgraph Legend
    direction LR
    _wf["Workflow/job"] ~~~ _cli(["CLI"]) ~~~ _logic["Logic"] ~~~ _api[["API"]] ~~~ _store[("Store")]
  end
Loading
High-Level Assessment

This is the right level to fix the issue: normalize the pnpm store path output at the boundary before calling @actions/cache, and validate it with a Windows E2E workflow. Alternatives like path.normalize() won’t remove the \\?\ prefix, and relying on pnpm configuration/environment to avoid extended-length paths is less reliable for consumers.

Files changed (3) +117 / -73

Bug fix (1) +19 / -1
run.tsNormalize Windows extended-length store paths before calling @actions/cache +19/-1

Normalize Windows extended-length store paths before calling @actions/cache

• Updates cache directory detection to strip the '\\?\' extended-length prefix returned by 'pnpm store path' on Windows. Adds a dedicated helper that also converts extended UNC paths ('\\?\UNC\...') back into regular UNC form ('\\server\share\...') to avoid '@actions/cache' glob parsing failures.

src/cache-restore/run.ts

Tests (1) +26 / -0
test.yamlAdd Windows E2E cache job for extended-length pnpm store paths +26/-0

Add Windows E2E cache job for extended-length pnpm store paths

• Introduces a 'cache-windows' job that forces pnpm’s store directory to the drive root to trigger an extended-length '\\?\' path. Runs the action with caching enabled and asserts 'pnpm store path' reports the expected extended-length prefix to ensure the regression is covered end-to-end.

.github/workflows/test.yaml

Other (1) +72 / -72
index.jsRebuild action bundle to include Windows cache path normalization +72/-72

Rebuild action bundle to include Windows cache path normalization

• Updates the compiled action bundle so runtime behavior matches the TypeScript source changes, including the extended-length path prefix removal logic used during cache restore.

dist/index.js

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/test.yaml:
- Line 106: Update the actions/checkout@v6 step in the pull-request workflow to
set persist-credentials to false. Preserve authenticated Git operations by
supplying a narrowly scoped token only to the specific steps that require it,
rather than retaining checkout credentials globally.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 390a73e6-93da-4374-b7cb-e9aa3494f960

📥 Commits

Reviewing files that changed from the base of the PR and between c9883cc and 20c4456.

⛔ Files ignored due to path filters (1)
  • dist/index.js is excluded by !**/dist/**
📒 Files selected for processing (2)
  • .github/workflows/test.yaml
  • src/cache-restore/run.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-11T16:19:49.450Z
Learnt from: zkochan
Repo: pnpm/setup PR: 1
File: src/cache-restore/run.ts:35-35
Timestamp: 2026-05-11T16:19:49.450Z
Learning: When using `actions/exec` (`getExecOutput` / `exec`), it is valid for the `commandLine` option to include both the command and its arguments in a single string (e.g., `getExecOutput('pnpm store path --silent')`). The library tokenizes `commandLine` internally (via `argStringToArray()`), so this behaves like passing an equivalent command + args array (e.g., `getExecOutput('pnpm', ['store','path','--silent'])`). In code reviews, do not flag this as incorrect—this matches documented behavior and a production-tested pattern.

Applied to files:

  • src/cache-restore/run.ts
🪛 zizmor (1.29.0)
.github/workflows/test.yaml

[warning] 106-106: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🔇 Additional comments (2)
src/cache-restore/run.ts (1)

42-63: LGTM!

.github/workflows/test.yaml (1)

99-105: LGTM!

Also applies to: 108-124

Comment thread .github/workflows/test.yaml
@zkochan
zkochan merged commit 34ec361 into main Aug 7, 2026
27 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.

1 participant