Skip to content

Invoke-Command $using improvements #24025

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 4, 2025

Conversation

jborean93
Copy link
Collaborator

PR Summary

Always use the v5.1 $using: logic when dealing with any connection type that is not based on the WSManConnectionInfo. This allows the caller to use the more advanced $using features like vars with special names, $using vars inside sub scopes and more. WSMan based connections might still communicate with older versions but as the oldest supported version is v3 on Server 2012 we don't need to keep the default as v2 to support some of the newer features.

PR Context

There are three different ways that Invoke-Command deals with $using: inside a ScriptBlock:

  • V2 - replaces the ScriptBlock text to change $using:var to $__using_var and modifies the param block to pass in that variable
  • V3/4 - builds an array containing the $using:var and passes it to the remote pipeline through --%
  • V5+ - like the above but uses a hashtable and has better support for $using: in child scopes

The V2 method is quite rudimentary, it fails when using a variable with an invalid char in like -, or in a very improbable scenario someone named their var as $__using_var themselves. The - problem is typically encountered when you try to inject a function into the scriptblock, for example this fails today

Function Test-Function { 'foo' }

Invoke-Command -HostName server {
    ${function:Test-Function} = ${using:function:Test-Function}
    
    Test-Function
}
At line:1 char:29
+ param($__using_function_Test-Function)
+                             ~
Missing ')' in function parameter list.

At line:1 char:38
+ param($__using_function_Test-Function)
+                                      ~
Unexpected token ')' in expression or statement.

At line:3 char:55
+     ${function:Test-Function} = $__using_function_Test-Function
+                                                       ~~~~~~~~~
Unexpected token '-Function' in expression or statement.

The workaround is to store the function in an intermediate var or define the function without - in the name:

Function Test-Function { 'foo' }

$testFunction = ${function:Test-Function}
Invoke-Command -HostName server {
    ${function:Test-Function} = ${using:testFunction}
    
    Test-Function
}

# or

Function Function { 'foo' }

Invoke-Command -HostName server {
    ${function:Test-Function} = ${using:function:Function}
    
    Test-Function
}

The V3/4, V5 logic works just fine with vars with special chars in the name

Function Test-Function { 'foo' }

$s = New-PSSession -UseWindowsPowerShell
Invoke-Command -Session $s {
    ${function:Test-Function} = ${using:function:Test-Function}
    
    Test-Function
}
$s | Remove-PSSession

The current logic for determining what logic to use is as follows

  • If the connection info is NewProcessConnectionInfo use V5
    • This only applies to Start-Job or if using -Session $s where the $s is from New-PSSession -UseWindowsPowerShell
  • If not an explicit PSSession through the -Session parameter use V2
  • If the PSSession supports disconnection operations
    • Use the PSVersionTable.PSVersion to determine the version
    • Only WSMan on Windows satisfies this check
  • All other scenarios default to V2

This means that V3/4, V5 is only used when the PSSession from New-PSSession -UseWindowsPowerShell or New-PSSession -ComputerName ... is given to Invoke-Command -Session .... The more common scenarios like Invoke-Command -ComputerName ... or Invoke-Command -HostName ... will always use the old V2 logic and thus cannot use the newer $using: features.

This PR simplifies the checks to set the baseline to V3/4 for a WSMan based connection and treat the rest of the connection types as V5. This is because only the WSMan connection was supported before 5.1 and can guarantee the target is PowerShell 5.1 or newer. The baseline for WSMan was set to V3/4 as V2 was last in box with Windows 7/Server 2008/08 R2 which is end of life. V3/4 is also strictly end of life but considering it's still supported in Azure I didn't think it was time to drop that just yet.

PR Checklist

@jborean93 jborean93 force-pushed the psremoting-v2 branch 3 times, most recently from aa0bdbc to c61690c Compare July 9, 2024 05:25
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Jul 16, 2024
@jborean93 jborean93 force-pushed the psremoting-v2 branch 2 times, most recently from fa5b5fa to af25933 Compare October 3, 2024 22:00
@SeeminglyScience SeeminglyScience added WG-Cmdlets general cmdlet issues WG-NeedsReview Needs a review by the labeled Working Group labels Dec 4, 2024
@SteveL-MSFT
Copy link
Member

The @PowerShell/wg-powershell-cmdlets discussed this. Although you have provided a technical solution, we don't believe this is a real world problem. We expect users to either install Windows PowerShell 5.1 or PowerShell 7 . Windows Server 2012/2012R2 end of life was Oct 2023. Windows Server 2016+ has 5.1. Alternatively, users who hit this problem likely already have implemented the workarounds. Although the change itself is small, we don't believe we should add support for PSv3/4 into PS7. It may make sense to explicitly add the workaround example in our documentation.

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Dec 18, 2024
@SteveL-MSFT SteveL-MSFT added Resolution-Declined The proposed feature is declined. WG-Reviewed A Working Group has reviewed this and made a recommendation and removed WG-NeedsReview Needs a review by the labeled Working Group labels Dec 18, 2024
@jborean93
Copy link
Collaborator Author

jborean93 commented Dec 18, 2024

@SteveL-MSFT I think you may have misunderstood what this PR is trying to do. The problem is that PowerShell today expects that the remote target is running v2 so that if $using:var is used it currently uses the older style Ast replacement method and injects the using vars as parameters to the scriptblock to be compatible with all possible target versions. What this PR does is

  • Sets the baseline the V3/4 compatible method for WSMan based connections
    • I didn't go with V5 out of caution of 2012/2012 R2 still being supported in Azure
    • I can even drop that if you wish based on your comments saying you just expect people to upgrade to 5.1 on those boxes
  • Sets the baseline to V5 for all other based connections
    • We can guarantee that these connections are at least 5.1

This does not fix support for older version but rather drops support for older versions so that when doing $using:foo it will support more complex scenarios like var names with - in them, nested variables inside ForEach-Object -Parallel blocks, etc.

@SteveL-MSFT SteveL-MSFT added WG-NeedsReview Needs a review by the labeled Working Group and removed Resolution-Declined The proposed feature is declined. WG-Reviewed A Working Group has reviewed this and made a recommendation labels Dec 18, 2024
@SteveL-MSFT
Copy link
Member

@jborean93 yes, it seems we misinterpreted the intent of this PR. I've marked it to discuss again in next WG meeting which will be mid-Jan

@jborean93
Copy link
Collaborator Author

jborean93 commented Dec 18, 2024

Thanks, while discussing it then could the WG decide whether the baseline for WSMan should be either the V3/4 one as done in this PR or V5? I stuck with V3/4 because 2012/2012 R2 is still supported in Azure and it solves the immediate problem of referencing functions with - in the $using: name but if you want to set V5 as the baseline for all that will certainly simplify the code and include some nested $using: fixes.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Dec 26, 2024
@SteveL-MSFT
Copy link
Member

The @PowerShell/wg-powershell-cmdlets discussed this again with the understanding that the intent is to remove support for v2 (which is completely out of support now) and enhance behavior for v3/4 so that users don't have to make the workaround which is not obvious. So there isn't a breaking change here since it used to not work (without the workaround) and now will work as expected from a PS7 client to a v3/4 target.

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Jan 15, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Jan 23, 2025
Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

Looks good. Left 1 comment.

Always use the v5.1 $using: logic when dealing with any connection type
that is not based on the WSManConnectionInfo. This allows the caller to
use the more advanced $using features like vars with special names,
$using vars inside sub scopes and more. WSMan based connections might
still communicate with older versions but as the oldest supported
version is v3 on Server 2012 we don't need to keep the default as v2 to
support some of the newer features.
@jborean93 jborean93 requested review from jshigetomi and a team as code owners July 29, 2025 19:32
@jborean93
Copy link
Collaborator Author

I've done a rebase push as this branch was very much out of date.

@daxian-dbw daxian-dbw self-assigned this Jul 29, 2025
Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

LGTM

@daxian-dbw daxian-dbw added CL-Engine Indicates that a PR should be marked as an engine change in the Change Log CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log and removed Review - Needed The PR is being reviewed CL-Engine Indicates that a PR should be marked as an engine change in the Change Log labels Jul 29, 2025
Co-authored-by: Travis Plunk <travis.plunk@microsoft.com>
@daxian-dbw daxian-dbw closed this Aug 4, 2025
@daxian-dbw daxian-dbw reopened this Aug 4, 2025
@daxian-dbw
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@daxian-dbw daxian-dbw merged commit 4415386 into PowerShell:master Aug 4, 2025
61 of 69 checks passed
Copy link
Contributor

microsoft-github-policy-service bot commented Aug 4, 2025

📣 Hey @@jborean93, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

@jborean93
Copy link
Collaborator Author

Thanks for the review/merge @daxian-dbw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log WG-Cmdlets general cmdlet issues WG-Reviewed A Working Group has reviewed this and made a recommendation
Projects
Development

Successfully merging this pull request may close these issues.

6 participants