Add code review instructions derived from PR feedback patterns by SteveL-MSFT · Pull Request #1646 · PowerShell/DSC

SteveL-MSFT · 2026-07-23T23:12:00Z

Copilot's automated code reviews sometimes flag false positives (e.g., claiming code won't compile when it does) or miss repo-specific patterns that human reviewers consistently catch. This PR adds a .github/instructions/code-review.instructions.md file to teach Copilot the recurring patterns and best practices specific to this repository.

Approach

Analyzed review comments across 30+ merged PRs spanning Dec 2025 -- Jul 2026, focusing on:

Human reviewer feedback that corrected Copilot or identified issues Copilot missed
Patterns where Copilot's feedback was accepted and led to fixes
Recurring themes in maintainer comments

The resulting instructions file is organized by domain and covers:

General principles -- avoid false compile-error claims, distinguish test harnesses from production code, recognize auto-generated files
Rust patterns -- cache key correctness, API visibility, fail-closed security, Windows FFI/COM safety (DLL loading, Drop, HRESULT), what-if correctness, Option return types
Pester test patterns -- cross-platform paths, env var isolation, ACL cleanup, exit code assertions, array comparisons, event subscriber cleanup, cmdlet skip guards
PowerShell adapter patterns -- module array returns, $using: in parallel blocks, terminating vs non-terminating errors
Design/architecture -- settings precedence chain, backward compatibility, semver conventions, canonical property naming, schema coherence
CI/CD -- fork permission limitations, conditional tool installation
Documentation -- debug print removal, locale string accuracy, dead i18n keys

Notes

The applyTo frontmatter targets **/*.rs, **/*.ps1, **/*.psm1, **/*.tests.ps1, **/*.json to cover the main file types in this repo
Instructions are phrased as actionable review guidance, not general coding advice
The "General Principles" section specifically addresses known false-positive patterns to reduce noise in future reviews

Analyzed review comments across 30+ PRs spanning Dec 2025 - Jul 2026 to extract recurring patterns and repo-specific best practices for code reviews: - General: avoid false-positive compile claims, respect test harness vs production distinction, recognize auto-generated files - Rust: cache keys, API visibility, fail-closed security, Windows FFI/COM safety, DLL loading security, what-if correctness, Option return types, schema coherence - Pester tests: cross-platform paths, env var isolation, ACL cleanup, exit code assertions, array comparisons, event subscriber cleanup, cmdlet skip guards - PowerShell adapters: module array returns, $using: in parallel blocks, terminating vs non-terminating error distinction - Design: settings precedence, backward compatibility, semver for resources, canonical property naming conventions, noFiltering semantics - CI/CD: fork permissions, conditional tool install, PS naming conventions - Docs: remove debug prints, locale string accuracy, dead i18n keys Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR adds a repository-specific GitHub Copilot code review instruction set under .github/instructions/ to reduce recurring false positives and encode patterns maintainers commonly enforce across DSC’s Rust, PowerShell/Pester, and JSON changes.

Changes:

Introduces a new Copilot code-review instruction file with guidance grouped by domain (Rust, PowerShell/Pester, architecture, CI/CD, docs/logging).
Adds targeted “avoid noise” principles (e.g., CI-validated compile/test claims, test-harness vs production expectations, autogenerated file handling).

github-actions · 2026-07-23T23:34:16Z

Code Coverage Report

No Rust files were changed in this PR.

🔵 Full Codebase Coverage

81% (good)

Metric	Value
Total executable lines	18217
Lines covered by tests	14904
Coverage percentage	81%

Full codebase coverage measures all instrumented Rust lines across the project.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Extracted and analyzed 2,733 review comments across all 455 PRs with reviews in this repository (Dec 2025 - Jul 2026). Used parallel analysis agents to synthesize patterns by theme: security, performance, testing, maintainability, and manifest/schema coherence. Key sections: - False-positive avoidance (patterns Copilot should NOT flag) - Security (fail-closed checks, DLL hijacking, policy bypass, secret leaks) - Rust patterns (caching, API visibility, error handling, FFI safety, what-if) - Pester testing (cross-platform, isolation, assertions, structure) - PowerShell adapters (module arrays, parallel scoping, error types) - Design (settings precedence, backward compat, schema coherence) - CI/CD (fork permissions, artifact contracts, control flow) - Documentation (comments, logging, i18n accuracy) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

…review-instructions

Reorganize the monolithic code-review.instructions.md into 11 focused instruction files under .github/instructions/code-review/, each with applyTo patterns targeting the relevant paths: - engine: configure, discovery, resource dispatch, functions, settings - resource: individual resource implementations - adapter: PowerShell/WMI adapter bridge code - extension: extension discovery and lifecycle - cli: command-line interface, subcommands, server mode - tests: Pester and Rust integration tests - library: shared libraries (dsc-lib, jsonschema, osinfo, etc.) - security: ACLs, policy, credentials, elevated resources - performance: allocations, caching, serialization hot paths - windows: FFI, COM, registry, DISM, services - linux: SSH config, apt/brew, platform-specific paths The top-level file now serves as a routing table mapping paths to areas, plus universal guidance (false-positive avoidance, documentation, CI/CD). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Move general adapter design principles (single-mode operation, input contract, structured I/O, exit codes, resource enumeration) to the top. Add explicit guidance that the validate operation is deprecated and adapters should rely on JSONSchema for validation. PowerShell-specific patterns are now in their own subsection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

.github/instructions/code-review.instructions.md:28

The routing table references pal/linux/, but there is no matching path in this repo (and pal/ only contains a Cargo.lock). This means Linux/macOS guidance may not be applied as intended; either remove this entry or update it to the actual platform-specific library path.

| Linux/macOS | `resources/apt/`, `resources/sshdconfig/`, `resources/brew/`, `pal/linux/` | `code-review/linux.instructions.md` |

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

.github/instructions/code-review/windows.instructions.md:2

applyTo includes pal/windows/**, but this repo has no pal/windows path (the PAL crate lives under lib/dsc-lib-pal/). As written, these Windows review instructions won’t apply to PAL code.

applyTo: 'resources/windows_firewall/**,resources/windows_service/**,resources/WindowsUpdate/**,resources/windows_personalization/**,resources/dism_dsc/**,resources/registry/**,lib/dsc-lib-registry/**,pal/windows/**'

.github/instructions/code-review/linux.instructions.md:3

applyTo references pal/linux/**, but there is no pal/linux in the repo. This means the Linux/macOS instructions won’t be routed based on that path, and the frontmatter description mentioning "platform abstraction" becomes misleading.

applyTo: 'resources/apt/**,resources/sshdconfig/**,pal/linux/**,resources/brew/**'
description: 'Code review guidance for Linux/macOS-specific code (SSH, apt, platform abstraction)'

.github/instructions/code-review.instructions.md:28

Area routing references pal/windows/ and pal/linux/, but those paths don’t exist in this repo. Also, the Windows routing row omits the existing resources/WindowsUpdate/ directory, which is included in the Windows-specific instruction file’s applyTo. Together, this can misroute reviewers and prevent the intended instruction sets from being applied.

| Windows | `resources/windows_*`, `resources/dism_dsc/`, `resources/registry/`, `lib/dsc-lib-registry/`, `pal/windows/` | `code-review/windows.instructions.md` |
| Linux/macOS | `resources/apt/`, `resources/sshdconfig/`, `resources/brew/`, `pal/linux/` | `code-review/linux.instructions.md` |

CLI additions must be reflected in the DSC server mode JSONRPC APIs. Rename section from 'Server Mode (MCP)' to 'Server Mode (JSONRPC)' for accuracy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

.github/instructions/code-review/windows.instructions.md:2

applyTo includes pal/windows/**, but there is no pal/windows/ directory in this repo (PAL code lives under lib/dsc-lib-pal/). As written, these Windows-specific review instructions won’t apply to the platform abstraction layer code.

applyTo: 'resources/windows_firewall/**,resources/windows_service/**,resources/WindowsUpdate/**,resources/windows_personalization/**,resources/dism_dsc/**,resources/registry/**,lib/dsc-lib-registry/**,pal/windows/**'

.github/instructions/code-review/linux.instructions.md:2

applyTo includes pal/linux/**, but there is no pal/linux/ directory in this repo. This makes the routing misleading and adds a dead pattern.

applyTo: 'resources/apt/**,resources/sshdconfig/**,pal/linux/**,resources/brew/**'

- Replace non-existent pal/windows/** and pal/linux/** with actual lib/dsc-lib-pal/** in applyTo patterns and routing table - Expand tests applyTo to include **/test/** and **/Tests/** directories (covers resources/apt/test/, adapters/powershell/Tests/, etc.) - Narrow println! guidance: debug println! should use tracing macros, but println! is acceptable for intentional CLI user-facing output Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

.github/instructions/code-review.instructions.md:27

The Area Routing table's Windows row uses resources/windows_*, but the repo also has a Windows resource at resources/WindowsUpdate/ (capitalized) which won't be implied by that pattern. Including it explicitly will make the routing guidance consistent with code-review/windows.instructions.md's applyTo, which already targets resources/WindowsUpdate/**.

| Windows | `resources/windows_*`, `resources/dism_dsc/`, `resources/registry/`, `lib/dsc-lib-registry/`, `lib/dsc-lib-pal/` | `code-review/windows.instructions.md` |

All Rust user-facing strings must use the t! macro from rust-i18n with text defined in en-us.toml rather than hard-coded in source files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Repetitive code should be flagged and a shared helper function suggested. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

.github/instructions/code-review.instructions.md:26

The Area Routing table says Performance guidance is only for "hot paths", but code-review/performance.instructions.md currently applies to all **/*.rs files. Align the routing description with the actual scope so reviewers (and Copilot routing) aren't misled.

| Performance | Any `*.rs` file in hot paths | `code-review/performance.instructions.md` |

- Performance instructions: clarify they apply to all Rust code (matching the applyTo pattern) but are most critical in hot paths - Compile-error principle: rephrase to evidence-based ('cite a specific rule violation') instead of relying on CI results the reviewer may not have in context Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 23, 2026 23:12

Copilot started reviewing on behalf of SteveL-MSFT July 23, 2026 23:16 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread .github/instructions/code-review.instructions.md

Comment thread .github/instructions/code-review.instructions.md Outdated

Copilot AI review requested due to automatic review settings July 24, 2026 00:52

Copilot started reviewing on behalf of SteveL-MSFT July 24, 2026 00:54 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

SteveL-MSFT marked this pull request as draft July 24, 2026 21:14

Copilot AI review requested due to automatic review settings July 25, 2026 15:58

SteveL-MSFT force-pushed the stevel-msft-add-code-review-instructions branch from 98036d8 to d2be216 Compare July 25, 2026 15:58

Copilot started reviewing on behalf of SteveL-MSFT July 25, 2026 15:59 View session

Copilot AI reviewed Jul 25, 2026

View reviewed changes

Comment thread .github/instructions/code-review.instructions.md Outdated

Comment thread .github/instructions/code-review.instructions.md Outdated

Comment thread .github/instructions/code-review.instructions.md Outdated

SteveL-MSFT and others added 2 commits July 25, 2026 11:01

Merge remote-tracking branch 'origin/main' into stevel-msft-add-code-…

a59d9c9

…review-instructions

Copilot AI review requested due to automatic review settings July 27, 2026 16:14

Copilot started reviewing on behalf of SteveL-MSFT July 27, 2026 16:15 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 27, 2026 16:21

Copilot started reviewing on behalf of SteveL-MSFT July 27, 2026 16:22 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Add CLI/server mode parity requirement to review instructions

a909210

CLI additions must be reflected in the DSC server mode JSONRPC APIs. Rename section from 'Server Mode (MCP)' to 'Server Mode (JSONRPC)' for accuracy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 27, 2026 17:01

Copilot started reviewing on behalf of SteveL-MSFT July 27, 2026 17:02 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread .github/instructions/code-review.instructions.md Outdated

Comment thread .github/instructions/code-review.instructions.md

Copilot AI review requested due to automatic review settings July 27, 2026 17:05

Copilot started reviewing on behalf of SteveL-MSFT July 27, 2026 17:06 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread .github/instructions/code-review/windows.instructions.md Outdated

Add rust-i18n t! macro requirement to general instructions

96801a1

All Rust user-facing strings must use the t! macro from rust-i18n with text defined in en-us.toml rather than hard-coded in source files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 27, 2026 17:13

Copilot started reviewing on behalf of SteveL-MSFT July 27, 2026 17:13 View session

SteveL-MSFT and others added 2 commits July 27, 2026 10:14

Add helper function extraction guidance to general principles

649e402

Repetitive code should be flagged and a shared helper function suggested. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Apply suggestions from code review

5fe848e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread .github/instructions/code-review/performance.instructions.md Outdated

Comment thread .github/instructions/code-review.instructions.md Outdated

Copilot AI review requested due to automatic review settings July 27, 2026 17:16

Copilot started reviewing on behalf of SteveL-MSFT July 27, 2026 17:17 View session

SteveL-MSFT marked this pull request as ready for review July 27, 2026 17:17

SteveL-MSFT requested review from michaeltlombardi and tgauth July 27, 2026 17:17

Copilot AI reviewed Jul 27, 2026

View reviewed changes

tgauth approved these changes Jul 27, 2026

View reviewed changes

Uh oh!

Conversation

SteveL-MSFT commented Jul 23, 2026

Approach

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Report

🔵 Full Codebase Coverage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 23, 2026 •

edited

Loading