Skip to content

fix: powershell WindmillClient module loading on Windows workers#8370

Merged
rubenfiszel merged 1 commit intomainfrom
fix-powershell-windmillclient-module-loa
Mar 14, 2026
Merged

fix: powershell WindmillClient module loading on Windows workers#8370
rubenfiszel merged 1 commit intomainfrom
fix-powershell-windmillclient-module-loa

Conversation

@rubenfiszel
Copy link
Contributor

Summary

  • Fix stale module cache detection: get_module_versions() previously returned ["unknown"] for empty/corrupted module directories, causing check_module_installed() to treat them as installed. Now verifies .psd1/.psm1 files actually exist in version subdirectories before considering a module cached.
  • Fix Import-Module regex: RE_POWERSHELL_IMPORTS used ^Import-Module which skipped indented imports. Changed to ^\s*Import-Module — the old behavior was masked by caching (module only needed to be detected once), but became visible when the cache path changed in 1.650.0.
  • Fix catch block formatting: Write-Output $_ was merged onto the same line as the closing " of the previous Write-Output due to a Rust line-continuation bug, causing PowerShell to parse $_ as an argument rather than a separate command. This is why error output showed the literal text Write-Output in the error message.

Regression introduced in: 1.650.0 (424ca59dfe — make WINDMILL_DIR configurable), which moved the Windows PowerShell cache from /tmp/windmill/cache/powershell to {temp_dir}/windmill/cache/powershell. The old cache was abandoned, and the bugs in module detection/regex prevented recovery.

Test plan

  • Windows worker: run a PowerShell script with Import-Module WindmillClient and verify it installs and loads correctly
  • Windows worker: run with indented Import-Module WindmillClient (inside function body or with leading spaces)
  • Verify error output no longer shows literal Write-Output text when a script fails
  • Delete the powershell cache directory and verify modules are re-downloaded on next run

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Mar 14, 2026

Claude finished @rubenfiszel's task in 54s —— View job


Code Review: PR #8370

  • Gather context and read changed files
  • Review code changes
  • Post review feedback

Overall: Clean, well-targeted bugfix addressing a regression from 1.650.0. All three changes are correct and the PR description is excellent — clear root cause analysis with the originating commit identified.

Fix 1: Regex for indented Import-Module (line 21)

-r#"^Import-Module\s+..."#
+r#"^\s*Import-Module\s+..."#

Correct fix. ^\s* allows leading whitespace so Import-Module inside function bodies or indented blocks is properly detected. No concerns.

Fix 2: Stale module cache detection (lines 199–213)

The old code pushed "unknown" when no version subdirectories were found, which made check_module_installed think the module was present. Now it verifies .psd1/.psm1 files actually exist before considering a version valid, and returns an empty vec for empty/corrupted directories.

This is the right approach — it's better to re-download than silently use a broken cache. One minor observation: the fs::read_dir + iterator chain is synchronous I/O on a tokio async context. This is fine in practice since this is a cache directory scan with very few entries, so blocking is negligible. No change needed.

Fix 3: Catch block formatting (lines 476–478)

-            Write-Output \"An error occurred:\n\"\
-            Write-Output $_
+            Write-Output \"An error occurred:\"\n\
+            Write-Output $_\n\

This is the most subtle fix. In the old code:

  • Line ending :\n\"\ — the \ at end of the Rust string literal continues to the next line, merging Write-Output "An error occurred:\n" with Write-Output $_ into a single line. PowerShell then interprets $_ as an argument to the first Write-Output rather than a separate command.
  • The fix correctly places \n\ (newline + line continuation) after each statement.

Correct and well-explained in the PR description.

No issues found

All three fixes are sound. The changes are minimal, focused, and address the root cause rather than symptoms.


Reproduction Instructions for Testing

On a Windows worker running Windmill 1.650.0+, create a new PowerShell script (Scripts > New Script > PowerShell). Write a script that uses Import-Module WindmillClient — both at the top level and indented inside a function body. First, delete any existing powershell cache directory under the worker's temp dir to start fresh. Run the script and verify: (1) WindmillClient installs and loads successfully, (2) indented Import-Module statements are detected, (3) if you intentionally throw an error, the error output shows the actual error message rather than literal Write-Output text in the catch block. Re-run the script to confirm the module is found in cache on subsequent runs.

@cloudflare-workers-and-pages
Copy link

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5431fa9
Status: ✅  Deploy successful!
Preview URL: https://ce626c23.windmill.pages.dev
Branch Preview URL: https://fix-powershell-windmillclien.windmill.pages.dev

View logs

@rubenfiszel rubenfiszel merged commit 3a268a9 into main Mar 14, 2026
14 checks passed
@rubenfiszel rubenfiszel deleted the fix-powershell-windmillclient-module-loa branch March 14, 2026 23:09
@github-actions github-actions bot locked and limited conversation to collaborators Mar 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant