-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Set-Content: avoid double confirmation by using CmdletProviderContext… #26548
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
base: master
Are you sure you want to change the base?
Conversation
…(ExecutionContext, CommandOrigin.Internal); add WhatIf test to assert single message (fixes PowerShell#24434)
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where Set-Content -Confirm prompted users twice for confirmation (issue #24434). The root cause was that both the cmdlet-level Set-Content and the provider-level ClearContent method called ShouldProcess, and the provider context inherited the cmdlet reference, causing double prompts.
The fix creates a new CmdletProviderContext using CommandOrigin.Internal instead of copying the existing context, which prevents the provider's ShouldProcess from prompting users while preserving essential properties like Filter and SuppressWildcardExpansion.
- Modifies
BeforeOpenStreamsto create a new context without the cmdlet reference - Adds a WhatIf test using transcript capture to verify single confirmation message
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Microsoft.PowerShell.Commands.Management/commands/management/SetContentCommand.cs | Creates a new CmdletProviderContext with CommandOrigin.Internal to avoid double ShouldProcess prompts when clearing content, while preserving essential context properties |
| test/powershell/Modules/Microsoft.PowerShell.Management/Set-Content.Tests.ps1 | Adds a test using Start-Transcript to verify Set-Content -WhatIf produces only one confirmation message |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Filter = currentContext.Filter, | ||
| }; |
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new context is missing several properties that may affect the provider's behavior during ClearContent:
- Force: Required for clearing content from hidden/readonly files (see FileSystemProvider.ClearContent line 7063)
- Include/Exclude: Filter patterns that may be needed by some providers
- DynamicParameters: Provider-specific parameters (e.g., -Stream for alternate data streams)
- Drive: The current drive context
- Credential: Authentication credentials if specified
These properties are set in the base class's CmdletProviderContext getter and should be copied to maintain consistent behavior. Consider copying these properties similar to how the CmdletProviderContext copy constructor does (CoreCommandContext.cs lines 265-275):
CmdletProviderContext context = new(currentContext.ExecutionContext, CommandOrigin.Internal)
{
SuppressWildcardExpansion = currentContext.SuppressWildcardExpansion,
Filter = currentContext.Filter,
Force = currentContext.Force,
DynamicParameters = currentContext.DynamicParameters,
Drive = currentContext.Drive
};
context.SetFilters(currentContext.Include, currentContext.Exclude, currentContext.Filter);| Filter = currentContext.Filter, | |
| }; | |
| Filter = currentContext.Filter, | |
| Force = currentContext.Force, | |
| DynamicParameters = currentContext.DynamicParameters, | |
| Drive = currentContext.Drive, | |
| Credential = currentContext.Credential | |
| }; | |
| context.SetFilters(currentContext.Include, currentContext.Exclude, currentContext.Filter); |
|
@mfloyd16 Please look failed tests. |
…Clear/Set-Content)
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
…(ExecutionContext, CommandOrigin.Internal); add WhatIf test to assert single message (fixes #24434)
PR Summary
Fixes: double confirmation prompts for Set-Content when -Confirm is used (issue #24434).
Cause: Provider-level ClearContent calls ShouldProcess while cmdlet-level Set-Content also calls ShouldProcess, and the provider context was inheriting the current cmdlet reference.
Result: Only one confirmation prompt is shown for Set-Content.
PR Context
This PR helps by making it more efficient for users running "Set-Content -Path file.txt -Value "content" -Confirm", previously you would have to confirm twice due to what seemed like a bug, now users only have to confirm once.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header