Skip to content

Conversation

@Borgquite
Copy link
Contributor

@Borgquite Borgquite commented Nov 18, 2025

Pull Request (PR) description

Adding various missing features to the module and doing a general tidy up with some bug fixes that have been encountered in production use.

  • UpdateServicesServer
    • BREAKING CHANGE: All parameters will now only be set when specifically applied
      rather than defaulting to hardcoded values if left undefined.
      In particular set ContentDir, Languages, Products, Classifications as needed.
  • UpdateServicesServer
    • Added support for the following settings:
      • ContentDir can be set to empty string for clients to download from Microsoft Update.
      • Updates are downloaded only when they are approved.
      • Express installation packages should be downloaded.
      • Update binaries are downloaded from Microsoft Update instead of from the
        upstream server.
        Fixes issue #39
      • WSUS infrastructure updates are approved automatically.
      • The latest revision of an update should be approved automatically.
      • An update should be automatically declined when it is revised to be expired
        and AutoRefreshUpdateApprovals is enabled.
      • The downstream server should roll up detailed computer and update status information.
      • Email status notifications and SMTP settings, including status notifications DST fix.
        Fixes issue #15
      • Use Xpress Encoding to compress update metadata.
      • Use foreground priority for BITS downloads
      • The maximum .cab file size (in megabytes) that Local Publishing will create.
      • The maximum number of concurrent update downloads.

Plus many bug fixes.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

@coderabbitai
Copy link

coderabbitai bot commented Nov 18, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request updates the UpdateServicesDsc module to add email/SMTP configuration support, improve parameter handling in UpdateServicesServer with a breaking change to apply only explicitly set parameters, fix product/language processing issues, add TimeOfDay testing in UpdateServicesCleanup, refactor UpdateServicesComputerTargetGroup with enhanced validation, and add module assertions across multiple resources for improved error handling.

Changes

Cohort / File(s) Summary
Schema and Configuration Additions
source/DSCResources/DSC_UpdateServicesServer/DSC_UpdateServicesServer.schema.mof
Added 18 new properties for email/SMTP configuration (SmtpHostName, SmtpPort, SenderDisplayName, SenderEmailAddress, EmailServerCredential, SmtpUserName, StatusNotificationFrequency, StatusNotificationTimeOfDay, StatusNotificationRecipients, SyncNotificationRecipients, EmailLanguage) and update download settings (DownloadUpdateBinariesAsNeeded, DownloadExpressPackages, GetContentFromMU, DoDetailedRollup, BitsDownloadPriorityForeground, IIsDynamicCompression, LocalPublishingMaxCabSize, MaxSimultaneousFileDownloads); enhanced descriptions for existing properties.
Localization Updates
source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
Extensive additions and renames of localization strings for new WSUS configuration options including GetWsusServerFailed, GettingWsusDatabaseConfig, GettingWsusEmailNotificationConfig, GettingWsusUpdateFiles, email and SMTP-related keys, synchronization keys, and multiple TestFailed entries.
Approval Rule Refactoring
source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
Added module assertion (Assert-Module UpdateServices) across all functions; reorganized error handling and initialization in Get-TargetResource; implemented WSUS role installation verification via registry check; refactored product matching logic to use GetUpdateCategories() and improved collection handling.
Cleanup Task TimeOfDay Validation
source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1, source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
Modified TimeOfDay extraction to parse StartBoundary as datetimeoffset; added TimeOfDay to Get-TargetResource return hashtable; implemented TimeOfDay comparison in Test-TargetResource with TimeOfDayTestFailed logging; added corresponding localization string.
ComputerTargetGroup Refactoring
source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
Added module assertion across all functions; refactored WSUS server existence and service installation checks; implemented parent target group path resolution and equality validation; added try/catch wrapping for CreateComputerTargetGroup and Delete operations with detailed error messaging and failure context reporting.
Changelog Documentation
CHANGELOG.md
Added Unreleased entries documenting breaking change for UpdateServicesServer parameter handling (issue #55), new settings and features, and multiple bug fixes across resources.
Minor Output Fix
source/Modules/PDT/PDT.psm1
Piped Wait-Win32ProcessStart output to Out-Null in Start-Win32Process to suppress output emission.

Sequence Diagram(s)

sequenceDiagram
    participant DSC as DSC Resource
    participant Assert as Assert-Module
    participant WSUS as WSUS Service
    participant Registry as Registry

    DSC->>Assert: Assert-Module UpdateServices
    DSC->>WSUS: Get-WsusServer
    alt Server Retrieved
        DSC->>Registry: Check UpdateServices role installed
        alt Role Installed
            DSC->>DSC: Initialize defaults
            DSC->>WSUS: Retrieve resource config
            DSC->>DSC: Return target resource state
        else Role Not Installed
            DSC->>DSC: Raise configuration error
        end
    else Server Not Available
        DSC->>DSC: Catch error, write verbose
        DSC->>DSC: Initialize defaults
    end
Loading
sequenceDiagram
    participant Test as Test-TargetResource
    participant CleanupTask as Cleanup Task
    participant TimeOfDay as TimeOfDay Param
    participant Validation as Validation Logic

    Test->>CleanupTask: Retrieve scheduled task
    alt Task Found
        Test->>CleanupTask: Extract StartBoundary
        Test->>Validation: Parse as datetimeoffset
        Test->>Validation: Extract TimeOfDay (c format)
        Test->>TimeOfDay: Compare with parameter
        alt Times Match
            Validation-->>Test: return true
        else Times Differ
            Validation->>Validation: Log TimeOfDayTestFailed
            Validation-->>Test: return false
        end
    else Task Not Found
        Test-->>Test: Handle absent state
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

  • DSC_UpdateServicesServer.schema.mof — 18 new properties added; verify property types, constraints, and descriptions align with WSUS API requirements and documented behavior.
  • DSC_UpdateServicesComputerTargetGroup.psm1 — Significant refactoring with path equality checks, parent group resolution, and detailed error messaging; validate logic flow for target group creation/deletion and edge cases (missing parents, duplicate groups).
  • DSC_UpdateServicesApprovalRule.psm1 — Product matching refactor using GetUpdateCategories(); ensure handling of duplicate product names and collection management is correct.
  • DSC_UpdateServicesCleanup.psm1 — TimeOfDay parsing from StartBoundary; verify datetimeoffset parsing and format conversion ('c' format) work correctly across locales and edge cases.
  • Localization strings (DSC_UpdateServicesServer.strings.psd1) — Extensive additions and renames; ensure consistency and completeness across all new features.

Possibly related PRs

Pre-merge checks

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main changes: a breaking change regarding parameter application and addition of new parameters, along with bug fixes.
Linked Issues check ✅ Passed Code changes address all linked issues: #15 (Email settings added to schema), #55 (parameter handling for undefined values), #63 (verbose output format fixed), #64 (Products processing improved), #76 (Languages type and verbose output fixed), and #93 (TimeOfDay testing added to UpdateServicesCleanup).
Out of Scope Changes check ✅ Passed All code changes are scoped to the linked issues and PR objectives: resource parameter additions, bug fixes, and verbose output corrections. The PDT.psm1 change (piping to Out-Null) is a minor fix supporting the overall objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description clearly relates to the changeset, describing multiple feature additions, breaking changes, bug fixes, and specific issues addressed across UpdateServicesServer and other components.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1 (1)

99-99: Localize the verbose message string.

The hard-coded string violates localization guidelines. All user-facing messages must use $script:localizedData.

As per coding guidelines.

Apply this diff:

-            Write-Verbose -Message 'Did not identify an instance of WSUS'
+            Write-Verbose -Message $script:localizedData.WsusNotIdentified

Then add the corresponding key to the localization file source/DSCResources/DSC_UpdateServicesApprovalRule/en-US/DSC_UpdateServicesApprovalRule.strings.psd1:

WsusNotIdentified = 'Did not identify an instance of WSUS'
🧹 Nitpick comments (10)
source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1 (1)

67-69: Consider the robustness of the registry check.

The hard-coded registry path and specific value check ('UpdateServices-Services' -eq '2') may be fragile if Microsoft changes the registry structure in future WSUS versions. Consider adding a comment explaining this check or verifying this approach against WSUS documentation.

source/DSCResources/DSC_UpdateServicesServer/DSC_UpdateServicesServer.schema.mof (4)

1-1: Consider bumping ClassVersion for this schema given the new properties

You’ve added a substantial set of properties and the resource behavior is changing in a breaking way; consider incrementing ClassVersion (e.g., to 2.0.0.0) so the schema version clearly reflects these changes.


18-20: Download behavior flags are well modelled; ensure examples/comment-based help are updated

The new toggles (DownloadUpdateBinariesAsNeeded, DownloadExpressPackages, GetContentFromMU) are sensibly named and typed. Just make sure the resource’s comment-based help and examples clearly explain how these interact with ContentDir and UpstreamServerName so users can reason about upstream/downstream and local vs MU content flows.


25-32: Documented value ranges should be enforced in the resource implementation

You’ve documented explicit ranges for SynchronizeAutomaticallyTimeOfDay (00:00:00–23:59:59) and SynchronizationsPerDay (1–24), and added new approval/targeting/rollup flags. Please ensure Test-TargetResource/Set-TargetResource validate these constraints and produce clear errors for invalid values; otherwise the schema description and actual behavior will diverge.


33-47: Align “omit for no notifications/anonymous” wording with new parameter semantics

The new email/SMTP-related properties are comprehensive and typed appropriately, but the descriptions for:

  • SyncNotificationRecipients / StatusNotificationRecipients (“omit for no notifications”)
  • EmailServerCredential (“omit for anonymous”)

may conflict with the new module-wide behavior where settings are only applied when parameters are explicitly provided. If omission now means “leave existing configuration as-is” rather than “force-disable”, the descriptions should be updated (and vice versa). It would also help to clarify how to clear existing recipients/credentials, if supported (e.g., empty array semantics).

While you’re here, consider simplifying the StatusNotificationFrequency description to avoid literal \n sequences if they show up verbatim in Get-DscResource output.

CHANGELOG.md (2)

10-14: Clarify the breaking-change wording and parameter list

The description correctly captures the new behavior (only explicitly specified parameters are applied) and ties to issue #55. To tighten the wording and make the “must set these now” aspect clearer, you could tweak the text slightly, for example:

-- UpdateServicesServer
-  - BREAKING CHANGE: All parameters will now only be set when specifically applied
-    rather than defaulting to hardcoded values if left undefined.
-    In particular set ContentDir, Languages, Products, Classifications as needed.
-    Fixes [issue #55](https://github.com/dsccommunity/UpdateServicesDsc/issues/55)
+- UpdateServicesServer
+  - BREAKING CHANGE: Parameters are now only applied when explicitly specified
+    instead of defaulting to hardcoded values when omitted.
+    In particular, set ContentDir, Languages, Products, and Classifications as needed.
+    Fixes [issue #55](https://github.com/dsccommunity/UpdateServicesDsc/issues/55).

Purely editorial, but this makes the breaking behavior and required parameters a bit more obvious.


44-62: New UpdateServicesServer settings read well; consider minor punctuation and consistency tweaks

The Added block maps nicely to the new capabilities (empty ContentDir, download behavior, infra auto-approval, rollup, email/SMTP, BITS, etc.) and the referenced issues (#39, #15) look accurate.

If you want to align punctuation and style with nearby bullets, you could make small adjustments:

-    - ContentDir can be set to empty string for clients to download from Microsoft Update.
+    - ContentDir can be set to an empty string for clients to download from Microsoft Update.
@@
-    - The downstream server should roll up detailed computer and update status information.
-    - Email status notifications and SMTP settings, including status notifications DST fix.
-      Fixes [issue #15](https://github.com/dsccommunity/UpdateServicesDsc/issues/15)
-    - Use Xpress Encoding to compress update metadata.
-    - Use foreground priority for BITS downloads
-    - The maximum .cab file size (in megabytes) that Local Publishing will create.
-    - The maximum number of concurrent update downloads.
+    - The downstream server should roll up detailed computer and update status information.
+    - Email status notifications and SMTP settings, including a status notifications DST fix.
+      Fixes [issue #15](https://github.com/dsccommunity/UpdateServicesDsc/issues/15).
+    - Use Xpress Encoding to compress update metadata.
+    - Use foreground priority for BITS downloads.
+    - The maximum .cab file size (in megabytes) that Local Publishing will create.
+    - The maximum number of concurrent update downloads.

Content itself looks good; these are just cosmetic improvements.

source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1 (2)

5-58: New WSUS configuration strings align well with the added settings

The added/expanded keys for server access, database/email config, upstream/replica/proxy, update files, auto-approvals, sync schedule, reporting rollup, and email notifications line up cleanly with the new UpdateServicesServer properties described in the PR (e.g., download binaries as needed, express packages, GetContentFromMU, auto approve infra updates, detailed rollup, email/SMTP settings, BITS foreground priority, local publishing limits, max simultaneous downloads). The naming is consistent with existing keys in this file.

One small grammar fix you might consider (to avoid a slightly jarring message at runtime):

-WsusUpdateFiles         = WSUS Server update files is content directory {0}, download binaries as needed {1}, download express packages {2}, get content from Microsoft Update {3}.
+WsusUpdateFiles         = WSUS Server update files are content directory {0}, download binaries as needed {1}, download express packages {2}, get content from Microsoft Update {3}.

Otherwise this block looks solid.


64-167: Configuring/TestFailed messages are thorough; minor optional wording polish

The new Configuring* and *TestFailed keys give good coverage of all the new behaviors (auto-approval, email notifications, sync schedule, rollup, IIS compression, BITS priority, local publishing, max downloads, etc.) and will be very helpful for debugging and verbose output.

If you want to tighten a couple of messages without changing semantics, here are optional tweaks:

-ConfiguringMaxSimultaneousFileDownloads = Configuring max simultaneous file downloads
+ConfiguringMaxSimultaneousFileDownloads = Configuring maximum simultaneous file downloads.

You might also scan this block once for consistent use of “e-mail” vs “email” and capitalization (“Synchronize Automatically” vs “synchronize automatically”), but that’s purely cosmetic and not required for functionality.

source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1 (1)

52-84: WSUS role verification and error handling look correct; consider a small style cleanup

The additional try/catch and the registry-based check for 'UpdateServices-Services' -eq '2' correctly gate WSUS access, avoid operating when the WSUS role is not installed, and map unexpected failures to the WSUSConfigurationFailed localized error. The duplicate group handling that compares the resolved Get-ComputerTargetGroupPath to the requested Path before marking Ensure = 'Present' is also consistent with the resource’s semantics.

One optional tweak: to align with the PowerShell guidelines, consider refactoring the multi-line if condition to avoid using backticks for line continuation (for example, assign the Get-ItemProperty result to a variable first, then keep the if expression on a single line).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d87a64e and 362350a.

📒 Files selected for processing (8)
  • CHANGELOG.md (3 hunks)
  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1 (5 hunks)
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1 (2 hunks)
  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1 (1 hunks)
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1 (3 hunks)
  • source/DSCResources/DSC_UpdateServicesServer/DSC_UpdateServicesServer.schema.mof (2 hunks)
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1 (2 hunks)
  • source/Modules/PDT/PDT.psm1 (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • Command: Public command
  • Function: Private function
  • Resource: DSC class-based resource

Build & Test Workflow Requirements

  • Run PowerShell script files from repository root
  • Setup build and test environment (once per pwsh session): ./build.ps1 -Tasks noop
  • Build project before running tests: ./build.ps1 -Tasks build
  • Always run tests in new pwsh session: Invoke-Pester -Path @({test paths}) -Output Detailed

File 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.ps1

Requirements

  • Follow instructions over existing code patterns
  • Follow PowerShell style and test guideline instructions strictly
  • 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
  • DSC resources should always be created as class-based resources
  • Add unit tests for all commands/functions/resources
  • Add integration tests for all public commands and resources

Files:

  • source/Modules/PDT/PDT.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
  • CHANGELOG.md
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
  • source/DSCResources/DSC_UpdateServicesServer/DSC_UpdateServicesServer.schema.mof
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
{**/*.ps1,**/*.psm1,**/*.psd1}

⚙️ CodeRabbit configuration file

{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell Guidelines

Naming

  • 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.ps1 format (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 array

Hashtables

  • 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...

Files:

  • source/Modules/PDT/PDT.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
source/DSCResources/**/*.psm1

⚙️ CodeRabbit configuration file

source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources Guidelines

Required Functions

  • Every DSC resource must define: Get-TargetResource, Set-TargetResource, Test-TargetResource
  • Export using *-TargetResource pattern

Function Return Types

  • Get-TargetResource: Must return hashtable with all resource properties
  • Test-TargetResource: Must return boolean ($true/$false)
  • Set-TargetResource: Must not return anything (void)

Parameter Guidelines

  • Get-TargetResource: Only include parameters needed to retrieve actual current state values
  • Get-TargetResource: Remove non-mandatory parameters that are never used
  • Set-TargetResource and Test-TargetResource: Must have identical parameters
  • Set-TargetResource and Test-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help comment

Required Elements

  • Each function must include Write-Verbose at least once
    • Get-TargetResource: Use verbose message starting with "Getting the current state of..."
    • Set-TargetResource: Use verbose message starting with "Setting the desired state of..."
    • Test-TargetResource: Use verbose message starting with "Determining the current state of..."
  • Use localized strings for all messages (Write-Verbose, Write-Error, etc.)
  • Import localized strings using Get-LocalizedData at module top

Error Handling

Files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
**/*.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 MD013 rule 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
  • Skip adding entry if same change already exists in Unreleased section
  • No duplicate sections or items in Unreleased section

Files:

  • CHANGELOG.md
🧠 Learnings (33)
📓 Common learnings
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:9-22
Timestamp: 2025-10-03T15:27:24.417Z
Learning: In the UpdateServicesDsc repository, MOF-based DSC resources follow a minimal comment-based help convention that includes only .SYNOPSIS and .PARAMETER sections. The .DESCRIPTION, .INPUTS, and .OUTPUTS sections are intentionally omitted, even though functions have [OutputType()] attributes. This is consistent across all existing DSC resources: MSFT_UpdateServicesServer, MSFT_UpdateServicesCleanup, and MSFT_UpdateServicesApprovalRule.
📚 Learning: 2025-10-04T21:33:23.022Z
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
  • CHANGELOG.md
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
  • source/DSCResources/DSC_UpdateServicesServer/DSC_UpdateServicesServer.schema.mof
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/[a-z][a-z]-[A-Z][A-Z]/DSC_*.strings.psd1 : Additional localization folders must be named using Get-UICulture names (e.g., fr-FR) and contain DSC_<ResourceName>.strings.psd1

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/en-US/DSC_*.strings.psd1 : Each resource directory must contain an en-US localization folder with a strings file named DSC_<ResourceName>.strings.psd1

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/[a-z][a-z]-[A-Z][A-Z]/DSC_*.strings.psd1 : In .strings.psd1 files, use underscores as word separators in localized string key names for multi-word keys

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use localized strings for all messages (e.g., Write-Verbose, Write-Error)

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/**/*.ps1 : Use localized string keys instead of hardcoded strings in script output/messages

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Store command/function localization in source/en-US/{MyModuleName}.strings.psd1

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Store class localization in source/en-US/{ResourceClassName}.strings.psd1

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Import localized strings at the top of the module using Get-LocalizedData

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Test-TargetResource must return a boolean ($true/$false)

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1
  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-10-03T15:27:24.417Z
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:9-22
Timestamp: 2025-10-03T15:27:24.417Z
Learning: In the UpdateServicesDsc repository, MOF-based DSC resources follow a minimal comment-based help convention that includes only .SYNOPSIS and .PARAMETER sections. The .DESCRIPTION, .INPUTS, and .OUTPUTS sections are intentionally omitted, even though functions have [OutputType()] attributes. This is consistent across all existing DSC resources: MSFT_UpdateServicesServer, MSFT_UpdateServicesCleanup, and MSFT_UpdateServicesApprovalRule.

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
  • CHANGELOG.md
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
  • source/DSCResources/DSC_UpdateServicesServer/DSC_UpdateServicesServer.schema.mof
  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Every MOF DSC resource module must define the functions: Get-TargetResource, Set-TargetResource, and Test-TargetResource

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Export resource functions using the *-TargetResource naming pattern

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Each of Get-/Set-/Test-TargetResource must include at least one Write-Verbose call

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-09-12T13:20:14.850Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-09-12T13:20:14.850Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Only classes decorated with [DscResource(...)] are in scope for these rules

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
📚 Learning: 2025-08-09T19:29:36.323Z
Learnt from: johlju
Repo: dsccommunity/DscResource.Test PR: 167
File: source/Private/Test-FileContainsClassResource.ps1:44-56
Timestamp: 2025-08-09T19:29:36.323Z
Learning: In the DscResource.Test repository, DSC resource attributes are consistently written as `[DscResource(...)]` rather than using variations like `[DscResourceAttribute()]` or fully qualified names like `[Microsoft.PowerShell.DesiredStateConfiguration.DscResource()]`. The Test-FileContainsClassResource function should focus on detecting the standard `[DscResource(...)]` pattern that is actually used in the codebase.

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Set-TargetResource and Test-TargetResource must have identical parameters

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Get-TargetResource: remove non-mandatory parameters that are never used

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
📚 Learning: 2025-08-17T10:48:15.384Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2136
File: source/suffix.ps1:24-24
Timestamp: 2025-08-17T10:48:15.384Z
Learning: In source/suffix.ps1, the Write-Verbose message in the catch block for Import-SqlDscPreferredModule does not need localization because the exception message from Import-SqlDscPreferredModule is already localized by that command, making it an edge case exception to the localization guidelines.

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-09-28T13:49:31.541Z
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/Get-ValidTimeSpan.ps1:36-39
Timestamp: 2025-09-28T13:49:31.541Z
Learning: In DSC Community guidelines, ErrorId parameters in New-TerminatingError calls are identifier strings that do not need localization - only the ErrorMessage parameter should use localized strings from $script:localizedData.

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/**/*.ps1 : When emitting messages, reference $script:localizedData.KeyName and format with the -f operator (e.g., Write-Verbose -Message ($script:localizedData.KeyName -f $value1))

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/**/*.ps1 : Assume and use $script:localizedData for accessing localized strings

Applied to files:

  • source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Get-TargetResource parameters: include only those needed to retrieve actual current state values

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Get-TargetResource must return a hashtable containing all resource properties

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Keep result capture and assertions in the same It block

Applied to files:

  • source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
📚 Learning: 2025-09-14T19:17:05.477Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-changelog.instructions.md:0-0
Timestamp: 2025-09-14T19:17:05.477Z
Learning: Applies to CHANGELOG.md : Describe notable changes briefly, with no more than 2 items per change type

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-11-14T08:43:03.398Z
Learnt from: raandree
Repo: dsccommunity/DscWorkshop PR: 193
File: build.yaml:34-42
Timestamp: 2025-11-14T08:43:03.398Z
Learning: In the dsccommunity/DscWorkshop repository, CHANGELOG.md is not currently maintained and should not be flagged as a required update.

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Set-TargetResource must not return anything (void)

Applied to files:

  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-10-10T14:01:42.703Z
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 86
File: tests/Unit/MSFT_UpdateServicesServer.Tests.ps1:6-45
Timestamp: 2025-10-10T14:01:42.703Z
Learning: For script-based (MOF) DSC resources in UpdateServicesDsc, unit tests should use Initialize-TestEnvironment with variables $script:dscModuleName and $script:dscResourceName, and set -ResourceType 'Mof'. This differs from class-based resource tests which use the simpler $script:moduleName template without Initialize-TestEnvironment.

Applied to files:

  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Test-TargetResource: Write-Verbose message should start with "Determining the current state of..."

Applied to files:

  • source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Include string IDs in the form (PREFIX####), where PREFIX is initials from the class/function name and numbers are sequential from 0001

Applied to files:

  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Name localization keys as Verb_FunctionName_Action using underscores (e.g., Get_Database_ConnectingToDatabase)

Applied to files:

  • source/DSCResources/DSC_UpdateServicesServer/en-US/DSC_UpdateServicesServer.strings.psd1
🪛 LanguageTool
CHANGELOG.md

[grammar] ~93-~93: Use a hyphen to join words.
Context: ...uages as a string array instead of comma separated values.
Fix issue [#76](h...

(QB_NEW_EN_HYPHEN)

🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md

95-95: Trailing spaces
Expected: 0 or 2; Actual: 4

(MD009, no-trailing-spaces)

🔇 Additional comments (13)
source/DSCResources/DSC_UpdateServicesCleanup/en-US/DSC_UpdateServicesCleanup.strings.psd1 (1)

14-14: LGTM!

The localization string is properly formatted and consistent with the existing test failure messages in this file.

source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1 (2)

90-90: LGTM!

The TimeOfDay property is correctly added to the return hashtable, consistent with the other cleanup task properties.


367-371: LGTM!

The TimeOfDay validation correctly implements the requirement from issue #93, ensuring the scheduled task is recreated when the time changes. The implementation is consistent with the other property validations in this function.

source/DSCResources/DSC_UpdateServicesApprovalRule/DSC_UpdateServicesApprovalRule.psm1 (7)

49-50: LGTM: Module assertion added.

The Assert-Module call ensures the UpdateServices module is loaded before attempting WSUS operations.


51-59: Verify Get-WsusServer error handling behavior.

The catch block only writes a verbose message without re-throwing or setting $WsusServer to $null explicitly. While the subsequent code checks for null, confirm this silent failure pattern aligns with the desired behavior when the WSUS server is unavailable.


60-64: LGTM: Default values initialized.

Initializing return values before the WSUS server check prevents undefined variable issues.


191-192: LGTM: Module assertion added.

The Assert-Module call ensures the UpdateServices module is loaded before attempting WSUS operations, consistent with Get-TargetResource and Test-TargetResource.


236-245: LGTM: Product matching logic correctly handles duplicate product names.

The updated logic retrieves all WSUS products once and uses Foreach-Object to add all matching products to the collection. This properly handles multiple product categories with the same name, addressing issue #64.


279-282: LGTM: Correctly removed -ErrorRecord parameter.

The -ErrorRecord $_ parameter was correctly removed since this code path is not within a catch block where $_ would be available.


405-406: LGTM: Module assertion added.

The Assert-Module call ensures the UpdateServices module is loaded before attempting WSUS operations, completing the consistent pattern across all three DSC resource functions.

source/DSCResources/DSC_UpdateServicesServer/DSC_UpdateServicesServer.schema.mof (2)

7-7: ContentDir description matches the new “empty string for MU” behavior

The updated ContentDir description clearly documents the empty-string semantic to route client downloads to Microsoft Update while keeping the type as String, which is appropriate. No schema changes needed here.


16-16: Read-only ProxyServerCredentialUsername looks appropriate

Introducing ProxyServerCredentialUsername as a [Read] string, separate from the MSFT_Credential write property, is a good pattern to surface the configured account without exposing secrets. This aligns well with how you handle EmailServerCredential/SmtpUserName later in the schema.

source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1 (1)

38-38: Module assertions improve reliability across all entry points

Adding Assert-Module -ModuleName UpdateServices at the start of Get-TargetResource, Set-TargetResource, and Test-TargetResource centralizes the dependency check and ensures the UpdateServices module is present before any WSUS calls run. This is a solid, low-cost safety improvement and looks correct.

Also applies to: 139-139, 274-274

}
}
$TimeOfDay = $Task.Triggers.StartBoundary.Split('T')[1]
$TimeOfDay = ([datetimeoffset]$Task.Triggers[0].StartBoundary).TimeOfDay.ToString('c')
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add error handling for potential null reference and parsing errors.

The code accesses $Task.Triggers[0] without verifying the array has elements and casts StartBoundary to [datetimeoffset] without error handling. Either condition could cause runtime errors.

Consider wrapping this in a try-catch block or adding validation:

-            $TimeOfDay = ([datetimeoffset]$Task.Triggers[0].StartBoundary).TimeOfDay.ToString('c')
+            if ($Task.Triggers -and $Task.Triggers.Count -gt 0)
+            {
+                try
+                {
+                    $TimeOfDay = ([datetimeoffset]$Task.Triggers[0].StartBoundary).TimeOfDay.ToString('c')
+                }
+                catch
+                {
+                    Write-Verbose -Message "Failed to parse TimeOfDay from scheduled task trigger."
+                }
+            }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In source/DSCResources/DSC_UpdateServicesCleanup/DSC_UpdateServicesCleanup.psm1
around line 69, the code directly indexes $Task.Triggers[0] and casts
StartBoundary to [datetimeoffset] which can throw null-reference or parse
exceptions; add validation to ensure $Task.Triggers is not null/empty and that
$Task.Triggers[0].StartBoundary is populated, and wrap the cast/parsing in a
try/catch (or use [datetimeoffset]::TryParse) to safely obtain TimeOfDay,
logging or defaulting when validation/parse fails so the function won’t error at
runtime.

throw $err
}
Wait-Win32ProcessStart @GetArguments
Wait-Win32ProcessStart @GetArguments | Out-Null
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Ignoring process startup validation could hide failures.

The return value from Wait-Win32ProcessStart indicates whether the process successfully started within the timeout period. By piping to Out-Null, this boolean is discarded, and the function continues to report success (line 724) even if the process failed to start.

This is inconsistent with Wait-Win32ProcessEnd (line 869), which properly checks the return value and throws an error on failure.

Apply this diff to validate the startup:

-        Wait-Win32ProcessStart @GetArguments | Out-Null
+        if (-not (Wait-Win32ProcessStart @GetArguments))
+        {
+            $errorMessage = (
+                $script:localizedData.ProcessFailedToStartError -f @($Path, $Arguments)
+            )
+            New-ArgumentException -ArgumentName 'Path' -Message $errorMessage
+        }

Note: Ensure ProcessFailedToStartError exists in your localization file (it's already used at line 872).

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In source/Modules/PDT/PDT.psm1 around line 716, the call "Wait-Win32ProcessStart
@GetArguments | Out-Null" discards the boolean result indicating whether the
process started; capture that return value into a variable, remove the Out-Null,
and immediately validate it (if the result is false) by throwing the localized
ProcessFailedToStartError (use the same localization pattern used elsewhere in
the file) so the function fails when the process does not start within the
timeout instead of proceeding as success.

@Borgquite Borgquite marked this pull request as draft November 18, 2025 17:27
@Borgquite
Copy link
Contributor Author

@codecov-ai-reviewer test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant