Fix Assert-ElevatedUser not terminating execution using ErrorActionPreference Stop #2125
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… execution Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Resolve the issue @dsccommunity/SqlServerDsc/issues/2070
Fix Assert-ElevatedUser not terminating execution in setup functions
Aug 1, 2025
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2125 +/- ##
====================================
Coverage 94% 94%
====================================
Files 127 127
Lines 8531 8543 +12
====================================
+ Hits 8078 8090 +12
Misses 453 453
🚀 New features to boost your workflow:
|
|
Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again. |
johlju
requested changes
Aug 30, 2025
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughMultiple cmdlets and private functions temporarily set and restore $ErrorActionPreference='Stop' around elevation checks and early validation to make non-terminating errors terminate. Changelog updated. New Pester test verifies Invoke-ReportServerSetupAction halts when Assert-ElevatedUser throws. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant F as Cmdlet/Function
participant AE as Assert-ElevatedUser
Note over F: New flow — temporarily set $ErrorActionPreference='Stop'
U->>F: Invoke cmdlet/function
F->>AE: Call Assert-ElevatedUser
AE-->>F: Throws terminating error (not elevated)
Note over F: Error is terminating and propagated
F-->>U: Terminating error returned (execution stops)
sequenceDiagram
autonumber
participant U as User
participant F as Cmdlet/Function
participant AE as Assert-ElevatedUser
Note over F: Previous flow — default ErrorActionPreference
U->>F: Invoke cmdlet/function
F->>AE: Call Assert-ElevatedUser
AE-->>F: Non-terminating error
alt Prior behavior
Note over F: Error may not have halted execution
F-->>U: Continued execution or non-terminating error output
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Copilot
AI
changed the title
Fix Assert-ElevatedUser not terminating execution in setup functions
Fix Assert-ElevatedUser not terminating execution by setting ErrorActionPreference
Aug 30, 2025
johlju
added
needs review
Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Contributor
Author
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/Public/Set-SqlDscStartupParameter.ps1 (1)
31-45: Fix incorrect command names in examplesExamples reference non-existent/old names (
Set-SqlDscStartupParameters,Set-SqlDscTraceFlag). This is user-facing and will confuse consumers.- Set-SqlDscStartupParameters -TraceFlag 4199 + Set-SqlDscStartupParameter -TraceFlag 4199 @@ - Set-SqlDscTraceFlag -ServiceObject $serviceObject -TraceFlag 4199 + Set-SqlDscStartupParameter -ServiceObject $serviceObject -TraceFlag 4199 @@ - Set-SqlDscTraceFlag -InstanceName 'SQL2022' -TraceFlag @() + Set-SqlDscStartupParameter -InstanceName 'SQL2022' -TraceFlag @()
🧹 Nitpick comments (4)
source/Public/Get-SqlDscStartupParameter.ps1 (2)
89-97: Prefer ThrowTerminatingError over Write-Error for explicit terminationFor the “failed to find service object” path, use
$PSCmdlet.ThrowTerminatingError()to produce an explicit terminating error with a precise category, instead of relying on$ErrorActionPreferenceto convertWrite-Errorinto terminating.- $writeErrorParameters = @{ - Message = $script:localizedData.StartupParameter_Get_FailedToFindServiceObject - Category = 'InvalidOperation' - ErrorId = 'GSDSP0001' # CSpell: disable-line - TargetObject = $ServiceObject - } - - Write-Error @writeErrorParameters + $PSCmdlet.ThrowTerminatingError( + [System.Management.Automation.ErrorRecord]::new( + $script:localizedData.StartupParameter_Get_FailedToFindServiceObject, + 'GSDSP0001', # CSpell: disable-line + [System.Management.Automation.ErrorCategory]::InvalidOperation, + $ServiceObject + ) + )
20-41: Nit: fix grammar in comment-based help examples (“command is run”)Minor wording tweak improves professionalism in help.
- Get the startup parameters from the Database Engine default instance on - the server where the command in run. + Get the startup parameters from the Database Engine default instance on + the server where the command is run. @@ - Get the startup parameters from the Database Engine default instance on - the server where the command in run. + Get the startup parameters from the Database Engine default instance on + the server where the command is run. @@ - Get the startup parameters from the Database Engine instance 'SQL2022' on - the server where the command in run. + Get the startup parameters from the Database Engine instance 'SQL2022' on + the server where the command is run. @@ - Get the startup parameters from the Database Engine instance 'SQL2022' on - the server where the command in run. + Get the startup parameters from the Database Engine instance 'SQL2022' on + the server where the command is run.source/Public/Set-SqlDscStartupParameter.ps1 (1)
126-134: Prefer ThrowTerminatingError over Write-Error for explicit terminationMatch the module’s pattern for terminating errors by using
$PSCmdlet.ThrowTerminatingError()rather than relying on$ErrorActionPreferenceto elevateWrite-Error.- $writeErrorParameters = @{ - Message = $script:localizedData.StartupParameter_Set_FailedToFindServiceObject - Category = 'InvalidOperation' - ErrorId = 'SSDSP0002' # CSpell: disable-line - TargetObject = $ServiceObject - } - - Write-Error @writeErrorParameters + $PSCmdlet.ThrowTerminatingError( + [System.Management.Automation.ErrorRecord]::new( + $script:localizedData.StartupParameter_Set_FailedToFindServiceObject, + 'SSDSP0002', # CSpell: disable-line + [System.Management.Automation.ErrorCategory]::InvalidOperation, + $ServiceObject + ) + )tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1 (1)
102-140: Make the assertion resilient and prove no further execution occurs.Prefer asserting on ErrorId (stable) over message text (localized/fragile), and mock Start-SqlSetupProcess to assert it’s not invoked after the terminating error.
Apply:
Context 'When user is not elevated' { BeforeAll { # Mock Assert-ElevatedUser to throw the same error it would in a real scenario Mock -CommandName Assert-ElevatedUser -MockWith { $PSCmdlet.ThrowTerminatingError( [System.Management.Automation.ErrorRecord]::new( 'This command must run in an elevated PowerShell session. (DRC0043)', 'UserNotElevated', [System.Management.Automation.ErrorCategory]::InvalidOperation, 'Command parameters' ) ) } + + # Ensure we can assert the command is not reached + Mock -CommandName Start-SqlSetupProcess -MockWith { 0 } -RemoveParameterValidation 'FilePath' # Create a valid executable file for the test New-Item -Path "$TestDrive/ssrs.exe" -ItemType File -Force | Out-Null @@ It 'Should throw a terminating error and not continue execution' { InModuleScope -ScriptBlock { # This test verifies the fix for issue #2070 where Assert-ElevatedUser # would throw an error but the function would continue executing { Invoke-ReportServerSetupAction @mockDefaultParameters } | - Should -Throw -ExpectedMessage '*This command must run in an elevated PowerShell session*' + Should -Throw -ErrorId 'UserNotElevated' } - # Ensure Assert-ElevatedUser was called + # Ensure Assert-ElevatedUser was called and Start-SqlSetupProcess was not Should -Invoke -CommandName Assert-ElevatedUser -Exactly -Times 1 -Scope It + Should -Invoke -CommandName Start-SqlSetupProcess -Exactly -Times 0 -Scope It } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)source/Private/Invoke-ReportServerSetupAction.ps1(1 hunks)source/Private/Invoke-SetupAction.ps1(1 hunks)source/Public/Get-SqlDscStartupParameter.ps1(1 hunks)source/Public/Set-SqlDscStartupParameter.ps1(1 hunks)tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run in PowerShell, from repository root
- Build before running tests:
.\build.ps1 -Tasks build- Always run tests in new PowerShell session:
Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow guidelines over existing code patterns
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Private/Invoke-ReportServerSetupAction.ps1tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1CHANGELOG.mdsource/Private/Invoke-SetupAction.ps1source/Public/Get-SqlDscStartupParameter.ps1source/Public/Set-SqlDscStartupParameter.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit configuration file
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: Lis...
Files:
source/Private/Invoke-ReportServerSetupAction.ps1tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1source/Private/Invoke-SetupAction.ps1source/Public/Get-SqlDscStartupParameter.ps1source/Public/Set-SqlDscStartupParameter.ps1
source/**/*.ps1
⚙️ CodeRabbit configuration file
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/{MyModuleName}.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_Database_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1source/Public/Get-SqlDscStartupParameter.ps1source/Public/Set-SqlDscStartupParameter.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Prefer
-BeTrue/-BeFalseover-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
MockinsideInModuleScope-blockFile Organization
- Class resources:
tests/Unit/Classes/{Name}.Tests.ps1- Public commands:
tests/Unit/Public/{Name}.Tests.ps1- Private functions:
tests/Unit/Private/{Name}.Tests.ps1Data-Driven Tests
- Define variables in separate
BeforeDiscoveryfor-ForEach(close to usage)-ForEachal...
Files:
tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.581Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction Stop on commands in integration tests so failures surface immediately
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Error for non-terminating errors with relevant error category
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1} : Use $PSCmdlet.ThrowTerminatingError() for terminating errors (except for classes) with relevant error category
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1} : Avoid empty catch blocks; prefer -ErrorAction SilentlyContinue
📚 Learning: 2025-08-29T17:24:23.694Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1} : Implement the Force parameter pattern: if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' }
Applied to files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1source/Public/Set-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-18T10:44:38.990Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: source/Public/New-SqlDscLogin.ps1:254-257
Timestamp: 2025-08-18T10:44:38.990Z
Learning: When implementing Force and Confirm parameter interaction in PowerShell functions with SupportsShouldProcess, use the pattern `if ($Force.IsPresent -and -not $Confirm)` to suppress confirmations. This ensures that explicit `-Confirm:$true` is respected even when `-Force` is present, preserving user intent when they explicitly request confirmation alongside Force.
Applied to files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1source/Public/Set-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-17T10:03:59.993Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:83-86
Timestamp: 2025-08-17T10:03:59.993Z
Learning: In PowerShell functions with SupportsShouldProcess, the $Confirm automatic variable defaults to $false when no -Confirm parameter is passed, and only becomes $true when -Confirm or -Confirm:$true is explicitly passed. The logic `if ($Force.IsPresent -and -not $Confirm)` correctly handles both cases where no -Confirm is passed and where -Confirm:$false is explicitly passed.
Applied to files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1source/Public/Set-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:24:23.694Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1} : Include a Force parameter for functions that use $PSCmdlet.ShouldContinue or $PSCmdlet.ShouldProcess
Applied to files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1source/Public/Set-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:24:23.694Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1} : Avoid empty catch blocks; prefer -ErrorAction SilentlyContinue
Applied to files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:20:42.238Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.238Z
Learning: Applies to tests/Stubs/SMO.cs : After changing SMO.cs (SMO stubs), run tests in a new PowerShell session
Applied to files:
tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1
📚 Learning: 2025-08-29T17:23:02.551Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.551Z
Learning: Applies to **/*.[Tt]ests.ps1 : Do not use Assert-MockCalled; use Should -Invoke instead
Applied to files:
tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1
📚 Learning: 2025-08-29T17:24:39.225Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:24:39.225Z
Learning: Applies to tests/[u]Unit/**/*.[Tt]ests.ps1 : Add Parameter Properties tests (e.g., assert Mandatory on parameters) as shown in the template
Applied to files:
tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1
📚 Learning: 2025-08-29T17:23:02.551Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.551Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use Mock inside an InModuleScope block
Applied to files:
tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.194Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.
Applied to files:
tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1
📚 Learning: 2025-08-29T17:24:39.225Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:24:39.225Z
Learning: Applies to tests/[u]Unit/**/*.[Tt]ests.ps1 : Mock file system interactions using the $TestDrive variable
Applied to files:
tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1
📚 Learning: 2025-08-29T17:22:07.581Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.581Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction Stop on commands in integration tests so failures surface immediately
Applied to files:
source/Private/Invoke-SetupAction.ps1source/Public/Get-SqlDscStartupParameter.ps1
📚 Learning: 2025-08-29T17:23:02.551Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.551Z
Learning: Applies to **/*.[Tt]ests.ps1 : Do not use Should -Not -Throw; invoke commands directly
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:21:35.556Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.556Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Do not use throw for terminating errors; use New-*Exception helpers (New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-25T10:07:22.349Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-08-25T10:07:22.349Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; handle errors via try/catch
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:24:23.694Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.694Z
Learning: Applies to **/*.{ps1,psm1} : For state-changing functions, use SupportsShouldProcess; place ShouldProcess check immediately before each state change; $PSCmdlet.ShouldProcess must follow the required pattern
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:21:35.556Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.556Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Use try/catch for error handling in resource methods
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:23:02.551Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.551Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:22:07.581Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.581Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : When calling commands in integration tests, pass -Force where applicable to avoid prompts
Applied to files:
source/Public/Set-SqlDscStartupParameter.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Publish Code Coverage)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (5)
source/Private/Invoke-ReportServerSetupAction.ps1 (1)
199-200: Good: escalate non-terminating errors to terminating within the function scopePlacing
$ErrorActionPreference = 'Stop'right after the parameter block ensures elevation failures and other non-terminating errors will halt execution before proceeding to confirmations or setup logic.CHANGELOG.md (1)
21-22: Changelog entry reads well and follows the repo’s styleThe fix is clearly described, references the issue, and uses inline code formatting appropriately.
source/Public/Get-SqlDscStartupParameter.ps1 (1)
68-69: Good: make errors terminate earlySetting
$ErrorActionPreference = 'Stop'beforeAssert-ElevatedUseraligns with the PR goal and prevents silent continuation.source/Public/Set-SqlDscStartupParameter.ps1 (1)
94-95: Good: terminate on errors at the start of executionSetting
$ErrorActionPreference = 'Stop'inbegin {}ensures early termination for elevation and discovery failures before any state changes.source/Private/Invoke-SetupAction.ps1 (1)
1375-1376: Terminating on non-terminating errors enabled (good placement).Setting $ErrorActionPreference = 'Stop' at the very start ensures Assert-ElevatedUser and subsequent calls halt correctly. Matches our guidelines and reviewer direction.
johlju
requested changes
Aug 30, 2025
Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Copilot
AI
changed the title
Fix Assert-ElevatedUser not terminating execution by setting ErrorActionPreference
Fix Assert-ElevatedUser not terminating execution while respecting ErrorAction parameter
Aug 30, 2025
Member
Co-authored-by: johlju <7189721+johlju@users.noreply.github.com>
Contributor
Author
Copilot
AI
changed the title
Fix Assert-ElevatedUser not terminating execution while respecting ErrorAction parameter
Fix Assert-ElevatedUser not terminating execution using ErrorActionPreference Stop
Aug 30, 2025
…o 'Stop' in multiple functions and restoring it afterward
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
source/Private/Invoke-SetupAction.ps1 (1)
1418-1418: Same optional suggestion as above regarding EAP scope/DRY.If you adopt a helper or single-scope convention in Invoke-ReportServerSetupAction, mirror it here for consistency.
🧹 Nitpick comments (2)
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md (2)
83-83: Clarify EAP guidance and fix MD013 (line-length).
- Wording implies -ErrorAction Stop also needs EAP=Stop, which is redundant and can confuse readers. Suggest clarifying that EAP=Stop is to convert non-terminating errors within the function scope.
- Wrap to ≤80 chars to satisfy markdownlint MD013.
Apply:
- When calling commands with `-ErrorAction 'Stop'`, temporarily set `$ErrorActionPreference = 'Stop'` and restore after the call. + Prefer setting `$ErrorActionPreference = 'Stop'` early in the function to + convert non-terminating errors to terminating within the function scope. + Restore it when intentionally limiting the scope (e.g., around a small block).
185-185: Align tone with list style and clarify behavior.Capitalize “Return” and be explicit to match surrounding bullets and prior guidance.
- - return `$null` for no objects/non-terminating errors + - Return `$null` when no objects are found or when a non-terminating error occurs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md(2 hunks)source/Private/Invoke-ReportServerSetupAction.ps1(2 hunks)source/Private/Invoke-SetupAction.ps1(2 hunks)source/Public/Get-SqlDscStartupParameter.ps1(1 hunks)source/Public/Set-SqlDscStartupParameter.ps1(1 hunks)tests/Unit/Public/Get-SqlDscStartupParameter.Tests.ps1(0 hunks)
💤 Files with no reviewable changes (1)
- tests/Unit/Public/Get-SqlDscStartupParameter.Tests.ps1
🚧 Files skipped from review as they are similar to previous changes (2)
- source/Public/Set-SqlDscStartupParameter.ps1
- source/Public/Get-SqlDscStartupParameter.ps1
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run in PowerShell, from repository root
- Build before running tests:
.\build.ps1 -Tasks build- Always run tests in new PowerShell session:
Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow guidelines over existing code patterns
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit configuration file
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: Lis...
Files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1
source/**/*.ps1
⚙️ CodeRabbit configuration file
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/{MyModuleName}.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_Database_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1
🧠 Learnings (31)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction Stop on commands in integration tests so failures surface immediately
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid empty catch blocks; prefer -ErrorAction SilentlyContinue
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.mdsource/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : .NOTES: include only critical info (constraints, side effects, security, version compatibility, breaking behavior) up to 2 short sentences
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:19:40.951Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T17:19:40.951Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Follow PowerShell code style guidelines in all PowerShell scripts, modules, and manifests
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Error for non-terminating errors with relevant error category
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:21:35.582Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.582Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Do not use throw for terminating errors; use New-*Exception helpers (New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.mdsource/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Use $PSCmdlet.ThrowTerminatingError() for terminating errors (except for classes) with relevant error category
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid global variables (exception: $global:DSCMachineStatus)
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-25T10:07:22.349Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-08-25T10:07:22.349Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; handle errors via try/catch
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.mdsource/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : For state-changing functions, use SupportsShouldProcess; place ShouldProcess check immediately before each state change; $PSCmdlet.ShouldProcess must follow the required pattern
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.mdsource/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Never use backtick (`) as line continuation in production code
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid Write-Output; use return instead
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Implement the ShouldProcess required message variables: $descriptionMessage, $confirmationMessage, $captionMessage as shown
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Limit piping to one pipe per line
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Avoid Write-Host; use Write-Verbose/Write-Information/etc.
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Use Write-Information for user-facing status updates, important operational messages, non-error state changes
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Include a Force parameter for functions that use $PSCmdlet.ShouldContinue or $PSCmdlet.ShouldProcess
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.mdsource/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Inside It blocks, assign unused return objects to $null unless part of a pipeline
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Assign function results to variables rather than inline calls
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Do not use unary comma operator in return statements to force an array
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-16T13:35:08.323Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2134
File: source/Public/Get-SqlDscLogin.ps1:0-0
Timestamp: 2025-08-16T13:35:08.323Z
Learning: In PowerShell, users expect to receive $null when no objects are found or when a non-terminating error occurs, rather than empty arrays. This is normal PowerShell behavior and should be maintained in DSC commands.
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Place $null on the left side of comparisons
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Use declared variables more than once (avoid single-use variables)
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Return a single, consistent object type per function
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-08-29T17:24:23.732Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-08-29T17:24:23.732Z
Learning: Applies to **/*.{ps1,psm1} : Implement the Force parameter pattern: if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' }
Applied to files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-18T10:44:38.990Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: source/Public/New-SqlDscLogin.ps1:254-257
Timestamp: 2025-08-18T10:44:38.990Z
Learning: When implementing Force and Confirm parameter interaction in PowerShell functions with SupportsShouldProcess, use the pattern `if ($Force.IsPresent -and -not $Confirm)` to suppress confirmations. This ensures that explicit `-Confirm:$true` is respected even when `-Force` is present, preserving user intent when they explicitly request confirmation alongside Force.
Applied to files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-17T10:03:59.993Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:83-86
Timestamp: 2025-08-17T10:03:59.993Z
Learning: In PowerShell functions with SupportsShouldProcess, the $Confirm automatic variable defaults to $false when no -Confirm parameter is passed, and only becomes $true when -Confirm or -Confirm:$true is explicitly passed. The logic `if ($Force.IsPresent -and -not $Confirm)` correctly handles both cases where no -Confirm is passed and where -Confirm:$false is explicitly passed.
Applied to files:
source/Private/Invoke-ReportServerSetupAction.ps1source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Do not use Should -Not -Throw; invoke commands directly
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction Stop on commands in integration tests so failures surface immediately
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:21:35.582Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-08-29T17:21:35.582Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Use try/catch for error handling in resource methods
Applied to files:
source/Private/Invoke-SetupAction.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Applied to files:
source/Private/Invoke-SetupAction.ps1
🪛 markdownlint-cli2 (0.17.2)
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
83-83: Line length
Expected: 80; Actual: 129
(MD013, line-length)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Publish Code Coverage)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (3)
source/Private/Invoke-ReportServerSetupAction.ps1 (2)
199-202: Good: Escalate to terminating errors for early validations.Saving/restoring EAP and setting it to 'Stop' before Assert-* aligns with the requested pattern and ensures elevation/parameter checks terminate.
233-234: Remove-SqlDscLogin.ps1 missing ErrorActionPreference save/restore: at line 113, add$previousErrorActionPreference = $ErrorActionPreferencebefore setting$ErrorActionPreference = 'Stop'and restore it after execution. Then extract the repeated save/set/restore logic into a helper and standardize naming ($previousErrorActionPreferencevs$originalErrorActionPreference) across all commands and resources.⛔ Skipped due to learnings
Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0 Timestamp: 2025-08-29T17:24:23.732Z Learning: Applies to **/*.{ps1,psm1} : Avoid empty catch blocks; prefer -ErrorAction SilentlyContinuesource/Private/Invoke-SetupAction.ps1 (1)
1375-1378: Good: Early EAP='Stop' before elevation and property asserts.Matches the Force/Confirm pattern and ensures non-terminating errors become terminating during validation.
…on setting `$ErrorActionPreference` before commands.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Assert-ElevatedUserfunction was not properly terminating execution when elevation checks failed. This fix ensures that functions properly terminate when elevation requirements are not met by setting$ErrorActionPreference = 'Stop'at the beginning of each function.Changes made:
$ErrorActionPreference = 'Stop'to the beginning of four functions to ensure proper error handlingAssert-ElevatedUser -ErrorAction 'Stop'now correctly terminates execution when elevation failsFiles modified:
source/Public/Get-SqlDscStartupParameter.ps1source/Public/Set-SqlDscStartupParameter.ps1source/Private/Invoke-SetupAction.ps1source/Private/Invoke-ReportServerSetupAction.ps1This approach ensures consistent error handling behavior throughout the module and properly addresses the elevation check requirements.
Fixes issue #2070 where Assert-ElevatedUser was not properly terminating execution.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
This change is