Skip to content

Add Shmuelie.Dsc: class-based DSC v3 resources for machine setup - #115

Merged
shmuelie merged 2 commits into
mainfrom
shmuelie/dsc-resources
Aug 24, 2026
Merged

Add Shmuelie.Dsc: class-based DSC v3 resources for machine setup#115
shmuelie merged 2 commits into
mainfrom
shmuelie/dsc-resources

Conversation

@shmuelie

@shmuelie shmuelie commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Adds a new Shmuelie.Dsc module of class-based DSC v3 resources for developer machine setup — the public-safe counterparts to a private set of setup resources. Each resource is a PowerShell class implementing Get()/Test()/Set(), exported via DscResourcesToExport.

Resources

Resource Key Purpose Depends on
SavePSResource Name Save a module to a local path (Save-PSResource, defaults to PSGallery; optional Version) Microsoft.PowerShell.PSResourceGet
SymbolicLink Path Create/verify a symbolic link built-in PowerShell
CopilotPlugin Source Install a GitHub Copilot CLI plugin public copilot CLI
CopilotMarketplace Name Register a Copilot CLI marketplace public copilot CLI
UvTool Name Install a Python tool (uv tool install) public uv CLI

Design details:

  • The Copilot and uv resources call the CLIs through small private wrappers, keeping the classes unit-testable. The wrappers set NO_COLOR/UV_NO_COLOR and strip ANSI escapes so colorized output doesn't defeat the checks.
  • Presence checks use whole-token matching over the CLI list output, so a desired name that is a substring of an installed one is not a false positive.
  • Get() reports actual observed state (a read-only Installed property on the CLI/save resources; the real link target on SymbolicLink).
  • CopilotPlugin derives the plugin name from owner/repo, plugin@marketplace, and market:plugin@marketplace; for URL sources set the optional Name property so the presence check matches.
  • Arguments passed to the CLIs are validated as shell-safe (guards the Windows .cmd/.bat re-parsing class), and Set() failures include the CLI's output.
  • Chosen as a single dedicated module (rather than folding DSC classes into the existing function modules) to keep those modules' shape unchanged.

Build / infra

  • Registered Shmuelie.Dsc in Build-Module.ps1, Publish-Module.ps1, Test-Modules.ps1, and the publish workflow (choice + tag regex).
  • Build-Module.ps1 now stages a module's Public folder only when it exists (so resource-only modules build) and fails fast if a module declares functions but has no Public folder.
  • Documentation site (docs/index.md, docs/modules.md) and root README updated.

Tests

tests/Shmuelie.Dsc.Tests.ps1 covers each resource's Get()/Test()/Set() logic (26 cases), including whole-token matching with substring non-matches, market:/URL name resolution, argument validation, ANSI stripping, and versioned SavePSResource. The tests mock the CLI wrappers, so they validate resource logic and command invocation — not live Copilot/uv output formats. Full suite: 335 passed, 0 failed; Test-Modules.ps1 green.

Scope

AgencyPlugin from the private set is intentionally excluded: it depends on internal-only tooling and has no public-safe form.

Closes #110
Closes #111
Closes #112
Closes #113
Closes #114

Introduce a new Shmuelie.Dsc module providing five class-based DSC v3 resources
for developer machine setup:

- SavePSResource: save a PowerShell module to a local path (Save-PSResource).
- SymbolicLink: create/verify a symbolic link to a target path.
- CopilotPlugin: install a GitHub Copilot CLI plugin.
- CopilotMarketplace: register a GitHub Copilot CLI marketplace.
- UvTool: install a Python tool via uv.

The Copilot and uv resources build on the public copilot and uv CLIs through
small private wrappers so the resource classes are unit-testable; the others
use built-in PowerShell only. The module is wired into the build, the publish
workflow, the documentation site, and the Pester suite.

Build-Module.ps1 now stages a module's Public folder only when it exists, so
resource-only modules (with no Public scripts) build.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b98f01a0-a039-492f-8576-b1917e54742d
@shmuelie

Copy link
Copy Markdown
Owner Author

Peer review — multi-persona panel (advisory)

Reviewed by a Code panel (Correctness, Skeptic, Craft, Compliance, Architect) plus a Writing pass on the description. Findings below are grouped by severity with the personas that raised each. The build and full test suite are green; these are correctness/robustness observations, not CI failures.

🔴 Blocking

1. Test() uses an unanchored substring match → silent false-positive, resource never converges. (Correctness, Skeptic; Craft raised it too)
CopilotPlugin, CopilotMarketplace, and UvTool check presence with [bool]($result.Output -match [regex]::Escape($name)). [regex]::Escape only escapes metacharacters — it adds no word/line boundaries, so any output line that contains the name as a substring matches. Test() then reports "in desired state" and Set() is skipped, so the item is never installed — with no error. Verified locally:

'fast-agent-mcp v1.2.3'      -match [regex]::Escape('mcp')    => True   # UvTool Name='mcp' → falsely "installed"
'dotnet-skills  dotnet/skills' -match [regex]::Escape('dotnet') => True   # marketplace/plugin 'dotnet' → falsely present

Recommendation: match whole tokens. For uv tool list (name starts the line): (?m)^$([regex]::Escape($name))(\s|$). For the Copilot lists: split each line and compare the first token with -eq. Add a regression test whose output contains a superstring of the desired name.

Coupled gap (Skeptic, Craft, Writing): the tests mock the Invoke-DscCopilot/Invoke-DscUv wrappers, so the matching logic is never exercised against real CLI output — which is why this bug passed 16 green tests. Add one characterization test per CLI resource using realistic (ANSI-stripped) output.

🟠 Important

2. Get() echoes desired state on 4 of 5 resources (DSC contract). (Correctness, Architect, Craft) SavePSResource/CopilotPlugin/CopilotMarketplace/UvTool Get() return the input properties without probing the system; only SymbolicLink.Get() reads real state. dsc resource get then can't distinguish present vs. absent, breaking drift reporting and any future Ensure: Absent. Rec: add an Installed/Ensure output property populated from $this.Test().

3. CopilotPlugin.Test() name derivation breaks URL and market: sources → re-installs every apply. (Correctness, Skeptic, Architect) ($Source -split '@')[0] -split '/' | Select -Last 1 yields my-plugin.zip for a URL and market:my-plugin for a market: spec (verified) — neither matches copilot plugin list, so Test() is always false. Rec: add an explicit optional Name property for those forms, or validate/reject unsupported Source shapes.

4. CLI output capture is fragile: 2>&1 + no color suppression. (Skeptic) Merging stderr into the match corpus can inject unrelated lines (and ErrorRecord objects) into the string match, and ANSI color codes (uv colorizes by default) make Test() false-negative. Rec: set NO_COLOR/UV_NO_COLOR, strip ANSI, keep stderr separate, and filter $Output to [string] before matching.

5. No validation of author-supplied args before a .cmd shim. (Skeptic) Source/Name/Repository flow straight into copilot/uv (which resolve to .cmd/.bat on Windows). The repo convention requires validating such values (BatBadBut / CVE-2024-1874). Author-controlled DSC properties lower the risk, but winget configure with community .dsc.yaml is a real vector. Rec: allow-list Source/Name/Repository before invoking the CLI.

🟡 Minor

  • 6. SavePSResource.Test() is version-blind (any version folder satisfies it); no Version property; PSResourceGet not in RequiredModules. (Skeptic)
  • 7. Build-Module.ps1's conditional Public/ copy loses fail-fast if an existing module loses its Public/. Consider asserting Public/ exists when FunctionsToExport is non-empty. (Skeptic)
  • 8. Set() failure messages discard $result.Output; include it for diagnosability. (Skeptic)
  • 9. SymbolicLink.Get() — the only stateful Get() — has no direct test for its three branches. (Architect, Craft)
  • 10. PR description could note the tests are mock-based and don't exercise live CLI parsing. (Writing)

✅ Verified solid / dismissed

  • Module boundary (dedicated Shmuelie.Dsc), build/publish/workflow wiring (ValidateSets + tag regex + docs), version-header/manifest consistency, empty aliases, comment-based help, and public-safety (no internal leakage) all check out. (Architect, Compliance)
  • $LASTEXITCODE capture in the wrappers and the [bool]($array -match …) coercion are correct. (Correctness)
  • One panelist flagged a missing module CHANGELOG.md as blocking — dismissed as inaccurate: the file exists and build/Test-Modules.ps1 (which enforces the per-module [Unreleased] changelog) passes.

Advisory review — nothing here gates the PR. The one blocking item (#1, with its test-coverage sibling) is the only "should fix before merge" in my view; the rest are quality/robustness improvements.

Address the peer-review findings on the DSC resources:

- Presence checks (CopilotPlugin/CopilotMarketplace/UvTool) now use whole-token
  matching over ANSI-stripped CLI output instead of an unanchored substring
  match, so a desired name that is a substring of an installed one no longer
  yields a false positive that silently skips installation.
- The CLI wrappers set NO_COLOR/UV_NO_COLOR, strip ANSI escapes, and coerce
  output to strings for robust parsing.
- Get() now reports actual state via a read-only Installed property (SymbolicLink
  already reported the real link target).
- CopilotPlugin resolves the plugin name from owner/repo, plugin@marketplace, and
  market:plugin@marketplace forms, and accepts an explicit Name for URL sources.
- CLI arguments are validated as shell-safe before invoking copilot/uv (guards
  the .cmd/.bat shim re-parsing class on Windows).
- SavePSResource gains an optional Version property (version-aware Test and Save).
- Set() failures include the CLI output in the error message.
- Build-Module.ps1 fails fast if a module declares functions but has no Public
  folder.

Tests expanded to 26 cases covering token matching (incl. substring
non-matches), Get()/Installed, URL/market: name resolution, argument
validation, ANSI stripping, and versioned SavePSResource.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b98f01a0-a039-492f-8576-b1917e54742d
@shmuelie

Copy link
Copy Markdown
Owner Author

Applied the review findings in 428ecb0: whole-token matching over ANSI-stripped output (blocking substring bug), stateful \Get()\ via a read-only \Installed\ property, \market:/URL name resolution with an explicit \Name, shell-safe argument validation, hardened CLI wrappers (NO_COLOR + string coercion), \SavePSResource\ \Version, CLI output in error messages, and a \Build-Module\ fail-fast. Tests expanded to 26 cases (incl. substring non-matches, \Get()/\Installed, arg validation, ANSI, versioned save); full suite 335 passed, 0 failed. The one dismissed finding (missing module CHANGELOG) remains a false positive — the file exists and Test-Modules enforces it.

@shmuelie
shmuelie merged commit e327f4f into main Aug 24, 2026
1 check passed
@shmuelie
shmuelie deleted the shmuelie/dsc-resources branch August 24, 2026 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment