Set-Content: avoid double confirmation by using CmdletProviderContext…#26548
Set-Content: avoid double confirmation by using CmdletProviderContext…#26548mfloyd16 wants to merge 3 commits intoPowerShell:masterfrom
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.
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, | ||
| }; |
There was a problem hiding this comment.
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