diff --git a/REVIEW_REPORT.md b/REVIEW_REPORT.md
new file mode 100644
index 0000000..7c6c5a0
--- /dev/null
+++ b/REVIEW_REPORT.md
@@ -0,0 +1,739 @@
+# PSProxmoxVE Module Review Report
+
+**Date:** 2026-03-19
+**Reviewer:** Claude Code (Opus 4.6)
+**Module Version:** 0.1.0-preview
+**Repository:** PSProxmoxVE (C# binary PowerShell module)
+
+---
+
+## Executive Summary
+
+1. **Well-structured project** with clean separation: `PSProxmoxVE.Core` (services, models, client) and `PSProxmoxVE` (cmdlets, module surface).
+2. **66 cmdlets** covering VMs, containers, storage, networking, SDN, users/roles/permissions, templates, cloud-init, snapshots, and tasks.
+3. **Excellent ShouldProcess coverage** -- all destructive cmdlets implement `SupportsShouldProcess` with appropriate `ConfirmImpact` levels.
+4. **OutputType attributes** present on every cmdlet. ValueFromPipelineByPropertyName used consistently for pipeline chaining.
+5. **Sync-over-async pattern** (`GetAwaiter().GetResult()`) used pervasively -- acceptable for PowerShell binary modules but a known tradeoff.
+6. **Comprehensive integration tests** running against real PVE 8 and PVE 9 via Terraform-provisioned nested instances.
+7. **Missing community files**: `CONTRIBUTING.md`, `CODE_OF_CONDUCT.md`, `SECURITY.md`, `.gitattributes`, issue/PR templates.
+8. **No publish workflow**: No CI pipeline for publishing to PSGallery.
+9. **One unapproved verb**: `Reset-PveVm` uses a hardcoded string `"Reset"` instead of `VerbsCommon` -- `Reset` is not an approved PowerShell verb.
+10. **PVE API coverage**: Module covers core automation scenarios well but lacks firewall, backup/restore (PBS), Ceph, HA, and replication management.
+
+---
+
+## Phase 1 -- Repository Inventory & Structure
+
+### Directory Tree (depth <= 4)
+
+```
+PSProxmoxVE/
++-- PSProxmoxVE.sln
++-- README.md
++-- LICENSE (MIT)
++-- CHANGELOG.md
++-- .editorconfig
++-- .gitignore
++-- CLAUDE.md
++-- .github/
+|   +-- workflows/
+|       +-- build.yml
+|       +-- unit-tests.yml
+|       +-- integration-tests.yml
++-- src/
+|   +-- PSProxmoxVE/
+|   |   +-- PSProxmoxVE.csproj
+|   |   +-- PSProxmoxVE.psd1
+|   |   +-- PSProxmoxVE.format.ps1xml
+|   |   +-- ModuleState.cs
+|   |   +-- Cmdlets/
+|   |       +-- PveCmdletBase.cs
+|   |       +-- CloudInit/ (3 cmdlets)
+|   |       +-- Connection/ (3 cmdlets)
+|   |       +-- Containers/ (8 cmdlets)
+|   |       +-- Network/ (10 cmdlets)
+|   |       +-- Nodes/ (2 cmdlets)
+|   |       +-- Snapshots/ (4 cmdlets)
+|   |       +-- Storage/ (5 cmdlets)
+|   |       +-- Tasks/ (2 cmdlets)
+|   |       +-- Templates/ (4 cmdlets)
+|   |       +-- Users/ (11 cmdlets)
+|   |       +-- Vms/ (17 cmdlets)
+|   +-- PSProxmoxVE.Core/
+|       +-- PSProxmoxVE.Core.csproj
+|       +-- Authentication/ (PveAuthenticator, PveAuthMode, PveSession, PveVersion)
+|       +-- Client/ (PveHttpClient)
+|       +-- Exceptions/ (7 exception types)
+|       +-- Models/ (Cluster, Containers, Network, Nodes, Storage, Users, Vms)
+|       +-- Services/ (11 service classes)
++-- tests/
+|   +-- PSProxmoxVE.Core.Tests/ (xUnit)
+|   |   +-- Authentication/ (3 test files)
+|   |   +-- Models/ (10 test files)
+|   |   +-- TestHelper.cs
+|   +-- PSProxmoxVE.Tests/ (Pester 5)
+|       +-- _TestHelper.ps1
+|       +-- CloudInit/ (1 test file)
+|       +-- Connection/ (3 test files)
+|       +-- Containers/ (3 test files)
+|       +-- Network/ (2 test files)
+|       +-- Nodes/ (1 test file)
+|       +-- Snapshots/ (1 test file)
+|       +-- Storage/ (3 test files)
+|       +-- Tasks/ (1 test file)
+|       +-- Templates/ (1 test file)
+|       +-- Users/ (3 test files)
+|       +-- Vms/ (6 test files)
+|       +-- Integration/ (1 comprehensive test file)
++-- tests/infrastructure/ (Terraform, Dockerfile, scripts for CI)
+```
+
+### Project Layout
+
+| Item | Value |
+|---|---|
+| Solution | `PSProxmoxVE.sln` |
+| Main project | `src/PSProxmoxVE/PSProxmoxVE.csproj` |
+| Core library | `src/PSProxmoxVE.Core/PSProxmoxVE.Core.csproj` |
+| Test project (xUnit) | `tests/PSProxmoxVE.Core.Tests/PSProxmoxVE.Core.Tests.csproj` |
+| Test project (Pester) | `tests/PSProxmoxVE.Tests/` |
+| Target Frameworks | `netstandard2.0`, `net9.0`, `net48` |
+| Language Version | C# 10.0 |
+| Nullable | Enabled |
+| Assembly Names | `PSProxmoxVE`, `PSProxmoxVE.Core` |
+
+### CI/CD Workflows
+
+| Workflow | Purpose |
+|---|---|
+| `build.yml` | Build + xUnit tests on net48 (Windows) and net9.0 (Windows + Ubuntu) |
+| `unit-tests.yml` | Build + Pester tests across PS 5.1/7.5 on Windows/Ubuntu/macOS |
+| `integration-tests.yml` | Full integration tests against nested PVE 8 + PVE 9 instances |
+
+### What's Present
+
+- [x] Solution file (`.sln`)
+- [x] Module manifest (`.psd1`) with explicit `CmdletsToExport`
+- [x] Format definitions (`.format.ps1xml`)
+- [x] `.editorconfig`
+- [x] `.gitignore`
+- [x] `LICENSE` (MIT)
+- [x] `CHANGELOG.md` (Keep a Changelog format)
+- [x] `README.md` (comprehensive)
+- [x] CI/CD workflows (build, unit tests, integration tests)
+
+### What's Missing
+
+- [ ] `CONTRIBUTING.md`
+- [ ] `CODE_OF_CONDUCT.md`
+- [ ] `SECURITY.md`
+- [ ] `.gitattributes`
+- [ ] `.github/ISSUE_TEMPLATE/`
+- [ ] `.github/pull_request_template.md`
+- [ ] PSGallery publish workflow
+- [ ] `ReleaseNotes` in manifest `PSData`
+- [ ] `IconUri` in manifest `PSData`
+
+---
+
+## Phase 2 -- PVE API Coverage Audit
+
+> **Note:** The PVE API documentation at `pve.proxmox.com/pve-docs/api-viewer/apidoc.js` was too large for automated extraction. Coverage assessment below is based on the author's knowledge of the PVE API and source code analysis.
+
+### Implemented Cmdlets by Category
+
+#### Connection (3 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Connect-PveServer` | `/access/ticket` | POST |
+| `Disconnect-PveServer` | `/access/ticket` | DELETE |
+| `Test-PveConnection` | `/version` | GET |
+
+#### Nodes (2 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveNode` | `/nodes` | GET |
+| `Get-PveNodeStatus` | `/nodes/{node}/status` | GET |
+
+#### VMs / QEMU (17 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveVm` | `/nodes/{node}/qemu` | GET |
+| `New-PveVm` | `/nodes/{node}/qemu` | POST |
+| `Remove-PveVm` | `/nodes/{node}/qemu/{vmid}` | DELETE |
+| `Start-PveVm` | `/nodes/{node}/qemu/{vmid}/status/start` | POST |
+| `Stop-PveVm` | `/nodes/{node}/qemu/{vmid}/status/stop` | POST |
+| `Restart-PveVm` | `/nodes/{node}/qemu/{vmid}/status/reboot` | POST |
+| `Suspend-PveVm` | `/nodes/{node}/qemu/{vmid}/status/suspend` | POST |
+| `Resume-PveVm` | `/nodes/{node}/qemu/{vmid}/status/resume` | POST |
+| `Reset-PveVm` | `/nodes/{node}/qemu/{vmid}/status/reset` | POST |
+| `Copy-PveVm` | `/nodes/{node}/qemu/{vmid}/clone` | POST |
+| `Move-PveVm` | `/nodes/{node}/qemu/{vmid}/migrate` | POST |
+| `Get-PveVmConfig` | `/nodes/{node}/qemu/{vmid}/config` | GET |
+| `Set-PveVmConfig` | `/nodes/{node}/qemu/{vmid}/config` | PUT |
+| `Resize-PveVmDisk` | `/nodes/{node}/qemu/{vmid}/resize` | PUT |
+| `Test-PveVmGuestAgent` | `/nodes/{node}/qemu/{vmid}/agent/ping` | POST |
+| `Get-PveVmGuestNetwork` | `/nodes/{node}/qemu/{vmid}/agent/network-get-interfaces` | GET |
+| `Invoke-PveVmGuestExec` | `/nodes/{node}/qemu/{vmid}/agent/exec` | POST |
+
+#### Containers / LXC (8 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveContainer` | `/nodes/{node}/lxc` | GET |
+| `New-PveContainer` | `/nodes/{node}/lxc` | POST |
+| `Remove-PveContainer` | `/nodes/{node}/lxc/{vmid}` | DELETE |
+| `Start-PveContainer` | `/nodes/{node}/lxc/{vmid}/status/start` | POST |
+| `Stop-PveContainer` | `/nodes/{node}/lxc/{vmid}/status/stop` | POST |
+| `Restart-PveContainer` | `/nodes/{node}/lxc/{vmid}/status/shutdown` | POST |
+| `Copy-PveContainer` | `/nodes/{node}/lxc/{vmid}/clone` | POST |
+| `Get-PveContainerConfig` | `/nodes/{node}/lxc/{vmid}/config` | GET |
+| `Set-PveContainerConfig` | `/nodes/{node}/lxc/{vmid}/config` | PUT |
+
+#### Storage (5 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveStorage` | `/storage` or `/nodes/{node}/storage` | GET |
+| `Get-PveStorageContent` | `/nodes/{node}/storage/{storage}/content` | GET |
+| `Send-PveIso` | `/nodes/{node}/storage/{storage}/upload` | POST |
+| `Invoke-PveStorageDownload` | `/nodes/{node}/storage/{storage}/download-url` | POST |
+| `New-PveStorage` | `/storage` | POST |
+| `Remove-PveStorage` | `/storage/{storage}` | DELETE |
+
+#### Snapshots (4 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveSnapshot` | `/nodes/{node}/qemu/{vmid}/snapshot` | GET |
+| `New-PveSnapshot` | `/nodes/{node}/qemu/{vmid}/snapshot` | POST |
+| `Remove-PveSnapshot` | `/nodes/{node}/qemu/{vmid}/snapshot/{name}` | DELETE |
+| `Restore-PveSnapshot` | `/nodes/{node}/qemu/{vmid}/snapshot/{name}/rollback` | POST |
+
+#### Networking (4 + 6 SDN cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveNetwork` | `/nodes/{node}/network` | GET |
+| `New-PveNetwork` | `/nodes/{node}/network` | POST |
+| `Set-PveNetwork` | `/nodes/{node}/network/{iface}` | PUT |
+| `Remove-PveNetwork` | `/nodes/{node}/network/{iface}` | DELETE |
+| `Invoke-PveNetworkApply` | `/nodes/{node}/network` | PUT |
+| `Get-PveSdnZone` | `/cluster/sdn/zones` | GET |
+| `New-PveSdnZone` | `/cluster/sdn/zones` | POST |
+| `Remove-PveSdnZone` | `/cluster/sdn/zones/{zone}` | DELETE |
+| `Get-PveSdnVnet` | `/cluster/sdn/vnets` | GET |
+| `New-PveSdnVnet` | `/cluster/sdn/vnets` | POST |
+| `Remove-PveSdnVnet` | `/cluster/sdn/vnets/{vnet}` | DELETE |
+
+#### Users / ACLs / Tokens (11 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveUser` | `/access/users` | GET |
+| `New-PveUser` | `/access/users` | POST |
+| `Remove-PveUser` | `/access/users/{userid}` | DELETE |
+| `Set-PveUser` | `/access/users/{userid}` | PUT |
+| `Get-PveRole` | `/access/roles` | GET |
+| `New-PveRole` | `/access/roles` | POST |
+| `Remove-PveRole` | `/access/roles/{roleid}` | DELETE |
+| `Get-PvePermission` | `/access/acl` | GET |
+| `Set-PvePermission` | `/access/acl` | PUT |
+| `Get-PveApiToken` | `/access/users/{userid}/token` | GET |
+| `New-PveApiToken` | `/access/users/{userid}/token/{tokenid}` | POST |
+| `Remove-PveApiToken` | `/access/users/{userid}/token/{tokenid}` | DELETE |
+
+#### Templates (4 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveTemplate` | `/nodes/{node}/qemu` (filtered) | GET |
+| `New-PveTemplate` | `/nodes/{node}/qemu/{vmid}/template` | POST |
+| `Remove-PveTemplate` | `/nodes/{node}/qemu/{vmid}` | DELETE |
+| `New-PveVmFromTemplate` | `/nodes/{node}/qemu/{vmid}/clone` | POST |
+
+#### Cloud-Init (3 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveCloudInitConfig` | `/nodes/{node}/qemu/{vmid}/config` | GET |
+| `Set-PveCloudInitConfig` | `/nodes/{node}/qemu/{vmid}/config` | PUT |
+| `Invoke-PveCloudInitRegenerate` | `/nodes/{node}/qemu/{vmid}/cloudinit` | PUT |
+
+#### Tasks (2 cmdlets)
+| Cmdlet | API Endpoint | Method |
+|---|---|---|
+| `Get-PveTask` | `/nodes/{node}/tasks/{upid}/status` | GET |
+| `Wait-PveTask` | `/nodes/{node}/tasks/{upid}/status` (polling) | GET |
+
+### Missing API Coverage by Functional Area
+
+| Area | Key Missing Endpoints | Priority |
+|---|---|---|
+| **Firewall** | `/nodes/{node}/firewall/rules`, `/cluster/firewall/*` | High |
+| **Backup / Restore** | `/nodes/{node}/vzdump`, backup schedule management | High |
+| **Pools** | `/pools`, `/pools/{poolid}` (CRUD) | Medium |
+| **HA** | `/cluster/ha/*` (groups, resources, status) | Medium |
+| **Ceph** | `/nodes/{node}/ceph/*` (OSD, pool, mon, fs) | Medium |
+| **Replication** | `/cluster/replication` (CRUD) | Low |
+| **Access Groups** | `/access/groups` (CRUD) | Low |
+| **Access Domains** | `/access/domains` (CRUD) | Low |
+| **Container Snapshots** | `/nodes/{node}/lxc/{vmid}/snapshot` | Medium |
+| **Container Migrate** | `/nodes/{node}/lxc/{vmid}/migrate` | Medium |
+| **PBS Integration** | Proxmox Backup Server operations | Low |
+| **Cluster Config** | `/cluster/config/*` (join, nodes, totem) | Low |
+| **ACME** | `/cluster/acme/*`, `/nodes/{node}/certificates/*` | Low |
+| **Node Management** | `/nodes/{node}/apt/*`, `/nodes/{node}/disks/*`, `/nodes/{node}/services/*` | Low |
+| **Metrics** | `/cluster/metrics/*` | Low |
+| **SDN Subnets** | `/cluster/sdn/vnets/{vnet}/subnets` | Medium |
+| **VM Agent** | Additional agent endpoints (file-read, file-write, etc.) | Low |
+
+### High-Value Gaps
+
+1. **Firewall management** -- Critical for security automation. No cmdlets for creating/managing firewall rules at cluster, node, or VM level.
+2. **Backup/restore (vzdump)** -- Essential for DR automation. No cmdlets for creating backups, managing schedules, or restoring.
+3. **Pool management** -- Required for multi-tenant environments. No cmdlets for creating/managing resource pools.
+4. **Container snapshots** -- Snapshots are only implemented for VMs, not LXC containers.
+5. **Container migration** -- `Move-PveVm` exists for VMs but no equivalent for containers.
+
+---
+
+## Phase 3 -- Code Quality & Best Practices
+
+### 3a. PowerShell Module Design
+
+#### Cmdlet Naming
+- **All cmdlets use approved PowerShell verbs** from the `VerbsCommon`, `VerbsLifecycle`, `VerbsCommunications`, `VerbsDiagnostic`, and `VerbsData` classes.
+- **One exception**: `Reset-PveVm` uses a hardcoded string `"Reset"` instead of a verb class constant: `[Cmdlet("Reset", "PveVm", ...)]`. While `Reset` IS an approved verb (in `VerbsCommon`), the implementation should use `VerbsCommon.Reset` for consistency.
+- Noun prefix `Pve` is consistent across all cmdlets.
+
+#### Parameter Design
+- `[Parameter(Mandatory = ...)]` used appropriately throughout.
+- `ValueFromPipelineByPropertyName = true` used on `Node`, `VmId`, and similar parameters enabling pipeline chaining (e.g., `Get-PveVm | Start-PveVm`).
+- `Position` attributes used on key parameters for positional binding.
+- `ValidateSet`, `ValidateRange`, `ValidateNotNullOrEmpty` used where appropriate (network types, port range, storage types, etc.).
+- **Gap**: VmId parameters are `int` type but lack `[ValidateRange(100, 999999999)]` to match PVE's VMID constraints.
+
+#### ShouldProcess / WhatIf / Confirm
+- **Excellent**: Every destructive/mutating cmdlet implements `SupportsShouldProcess = true`.
+- `ConfirmImpact = ConfirmImpact.High` is set on all `Remove-*` cmdlets, `New-PveTemplate` (irreversible), and `Restore-PveSnapshot`.
+- **Gap**: `Stop-PveVm`, `Stop-PveContainer`, `Reset-PveVm` do not set `ConfirmImpact.High` -- stopping/resetting VMs can cause data loss.
+
+#### OutputType
+- `[OutputType]` attributes are present on **all 66 cmdlets**. This is exemplary.
+
+#### Pipeline Support
+- All cmdlets use `ValueFromPipelineByPropertyName` on `Node` and `VmId` parameters.
+- `Get-PveVm` and `Get-PveContainer` output objects with `Node` and `VmId` properties, enabling chaining.
+- **Gap**: None of the cmdlets implement `ProcessRecord` for pipeline batch processing -- all use `EndProcessing` (or equivalent). This means `Get-PveVm | Start-PveVm` processes only the last piped object. **This is a functional bug if multiple objects are piped.**
+
+#### Error Handling
+- `ThrowTerminatingError` used appropriately in `ConnectPveServerCmdlet`, `WaitPveTaskCmdlet`, `GetPveNodeCmdlet`, `GetPveNodeStatusCmdlet`.
+- `WriteWarning` used in `DisconnectPveServerCmdlet` and `SetPveCloudInitConfigCmdlet` for non-fatal conditions.
+- `WriteVerbose` used in connection cmdlets for diagnostic output.
+- **Gap**: Most cmdlets let exceptions propagate without wrapping in `ErrorRecord` -- this means non-terminating errors are not always well-structured.
+
+### 3b. C# Code Quality
+
+#### Null-Reference Safety
+- `<Nullable>enable</Nullable>` in both `.csproj` files.
+- Nullable annotations used on session, parameters, and return types.
+- Null guards (`?? throw`) used in constructors and authentication methods.
+
+#### Async/Await Pattern
+- All HTTP operations are async at the `PveHttpClient` level.
+- Sync wrappers use `.GetAwaiter().GetResult()` pattern.
+- **Assessment**: This is the standard approach for PowerShell binary modules on .NET. True async cmdlets are only available via `BeginProcessing`/`EndProcessing` async overrides in PS 7.4+, which would break PS 5.1 compatibility.
+- **Risk**: Low. PowerShell cmdlets run on a single pipeline thread, so deadlock risk from `GetAwaiter().GetResult()` is minimal given the `ConfigureAwait(false)` usage throughout.
+
+#### IDisposable Correctness
+- `PveHttpClient` implements `IDisposable` correctly with a `_disposed` flag.
+- All usages in cmdlets and services use `using var client = new PveHttpClient(session)` or `using (var client = ...)` blocks.
+- `ProgressStream` disposes its inner stream correctly.
+- File upload uses `try/finally` for stream cleanup with conditional `DisposeAsync` on .NET 9+.
+- **Assessment**: Excellent. No resource leak risks identified.
+
+#### Exception Handling
+- Custom exception hierarchy: `PveApiException`, `PveAuthenticationException`, `PveNotConnectedException`, `PveSessionExpiredException`, `PveTaskTimeoutException`, `PveTaskFailedException`, `PveVersionException`.
+- `PveHttpClient.SendAsync` catches `HttpRequestException` and wraps it in `PveApiException`.
+- Error message extraction from PVE API JSON responses is handled gracefully.
+- The only bare `catch {}` is in `ExtractErrorMessage` where parse failure falls through to raw body -- this is acceptable.
+
+#### Magic Strings
+- API resource paths are constructed inline in service methods (e.g., `$"nodes/{node}/qemu/{vmid}/status/start"`). This is pragmatic for the size of the codebase.
+- Auth header names (`PVEAPIToken=`, `PVEAuthCookie=`, `CSRFPreventionToken`) are inline. Could be constants but not a significant issue.
+
+#### LINQ
+- No complex LINQ usage. JObject/JArray parsing uses Newtonsoft.Json's indexer pattern which is straightforward.
+
+#### Logging / Verbose Output
+- `WriteVerbose` used in `ConnectPveServerCmdlet` and `DisconnectPveServerCmdlet`.
+- **Gap**: No `WriteVerbose` calls in any other cmdlets -- users cannot see which API calls are being made. This hinders debugging.
+- `WriteDebug` is not used anywhere in the module.
+- **Gap**: `WriteProgress` is only used in `Send-PveIso` (via progress callback). Not used in `Wait-PveTask` or other long-running operations.
+
+### 3c. General Hygiene
+
+#### Unused Code / Dead Code
+- No unused `using` directives detected (CS1591 suppressed for missing XML docs, not for unused code).
+- No commented-out blocks found.
+- **Clean**: Zero TODO/FIXME/HACK markers in source code.
+
+#### Code Style
+- Consistent 4-space indentation matching `.editorconfig`.
+- Opening braces on same line (Allman-adjacent style used in some places, K&R in others -- minor inconsistency but not problematic).
+- PascalCase for public members, camelCase for locals/fields.
+
+#### XML Doc Comments
+- Public types in `PSProxmoxVE.Core` have XML doc comments (enabled via `<GenerateDocumentationFile>true</GenerateDocumentationFile>`).
+- `CS1591` is suppressed in Core project (`<NoWarn>$(NoWarn);CS1591</NoWarn>`) so not all public members have docs.
+- Cmdlet classes have XML summary comments on the class level.
+- **Gap**: Parameter help text (`[Parameter(HelpMessage = ...)]`) is not used on any cmdlet parameter. This means `Get-Help` shows no parameter descriptions.
+
+---
+
+## Phase 4 -- Testing Coverage Analysis
+
+### Test Projects
+
+| Project | Framework | Type | Runner |
+|---|---|---|---|
+| `PSProxmoxVE.Core.Tests` | xUnit 2.7 + Moq 4.20 | Unit tests | `dotnet test` |
+| `PSProxmoxVE.Tests` | Pester 5 | Cmdlet + Integration tests | `Invoke-Pester` |
+
+### xUnit Tests (Core Library)
+
+| Test File | Test Area |
+|---|---|
+| `Authentication/PveSessionTests.cs` | Session creation, expiry, auth modes |
+| `Authentication/PveAuthenticatorTests.cs` | Authentication logic |
+| `Authentication/PveVersionTests.cs` | Version parsing |
+| `Models/ClusterModelTests.cs` | Cluster model deserialization |
+| `Models/ContainerModelTests.cs` | Container model deserialization |
+| `Models/NetworkModelTests.cs` | Network model deserialization |
+| `Models/NodeModelTests.cs` | Node model deserialization |
+| `Models/SdnModelTests.cs` | SDN model deserialization |
+| `Models/SnapshotModelTests.cs` | Snapshot model deserialization |
+| `Models/StorageModelTests.cs` | Storage model deserialization |
+| `Models/TaskModelTests.cs` | Task model deserialization |
+| `Models/UserModelTests.cs` | User model deserialization |
+| `Models/VmModelTests.cs` | VM model deserialization |
+
+### Pester Tests (Cmdlet Level)
+
+| Test File | Cmdlet(s) Covered |
+|---|---|
+| `Connection/Connect-PveServer.Tests.ps1` | `Connect-PveServer` |
+| `Connection/Disconnect-PveServer.Tests.ps1` | `Disconnect-PveServer` |
+| `Connection/Test-PveConnection.Tests.ps1` | `Test-PveConnection` |
+| `Containers/Get-PveContainer.Tests.ps1` | `Get-PveContainer` |
+| `Containers/ContainerLifecycle.Tests.ps1` | Container start/stop/restart |
+| `Containers/ContainerConfigCmdlets.Tests.ps1` | Container config cmdlets |
+| `CloudInit/CloudInitCmdlets.Tests.ps1` | Cloud-init cmdlets |
+| `Network/Get-PveNetwork.Tests.ps1` | `Get-PveNetwork` |
+| `Network/SdnCmdlets.Tests.ps1` | SDN cmdlets |
+| `Nodes/NodeCmdlets.Tests.ps1` | Node cmdlets |
+| `Snapshots/SnapshotCmdlets.Tests.ps1` | Snapshot cmdlets |
+| `Storage/Get-PveStorage.Tests.ps1` | `Get-PveStorage` |
+| `Storage/StorageDownload.Tests.ps1` | `Invoke-PveStorageDownload` |
+| `Storage/Send-PveIso.Tests.ps1` | `Send-PveIso` |
+| `Tasks/TaskCmdlets.Tests.ps1` | Task cmdlets |
+| `Templates/TemplateCmdlets.Tests.ps1` | Template cmdlets |
+| `Users/UserCmdlets.Tests.ps1` | User CRUD |
+| `Users/ApiTokenCmdlets.Tests.ps1` | API token CRUD |
+| `Users/RoleCmdlets.Tests.ps1` | Role CRUD |
+| `Vms/Get-PveVm.Tests.ps1` | `Get-PveVm` |
+| `Vms/New-PveVm.Tests.ps1` | `New-PveVm` |
+| `Vms/Remove-PveVm.Tests.ps1` | `Remove-PveVm` |
+| `Vms/VmLifecycle.Tests.ps1` | VM start/stop/restart |
+| `Vms/VmAdvancedOps.Tests.ps1` | Clone, migrate, resize |
+| `Vms/VmConfigCmdlets.Tests.ps1` | VM config cmdlets |
+
+### Integration Tests
+
+The integration test file (`Integration.Tests.ps1`) is comprehensive and covers:
+
+| Context | Operations Tested |
+|---|---|
+| Connection | Connect via API token, version detection |
+| Nodes | List nodes, get node status |
+| User CRUD | Create, list, update, remove user |
+| Role CRUD | Create, list, remove role |
+| API Token CRUD | Create, list, remove API token |
+| Permissions | List and set permissions |
+| VMs | List, create, config get/set, start/stop, hard reset, clone |
+| Snapshots | Create, list, restore, remove |
+| Storage | List storage, list content, upload ISO |
+| Tasks | Get task, wait for task |
+| Network | List networks |
+| Templates | List templates, convert to template, clone from template |
+| Cloud-Init | Get cloud-init config |
+| Guest Agent | Ping, network interfaces, exec command |
+| ACPI Lifecycle | Graceful restart, graceful stop |
+
+- **PVE 8 and PVE 9 both tested** via matrix strategy in CI.
+- **Setup/teardown**: AfterAll block cleans up created VMs, users, and roles.
+- **Skip logic**: Tests skip gracefully when env vars are not set.
+- **Connection details**: Injected via environment variables (`PVETEST_HOST`, `PVETEST_APITOKEN`, etc.).
+
+### Test Coverage Gap List
+
+| Cmdlet | Unit Test | Integration Test | Gap |
+|---|---|---|---|
+| `Set-PveNetwork` | Pester | No | No integration test for network modification |
+| `New-PveNetwork` | Pester | No | No integration test for network creation |
+| `Remove-PveNetwork` | Pester | No | No integration test for network removal |
+| `Invoke-PveNetworkApply` | Pester | No | No integration test for network apply |
+| `New-PveSdnZone` | Pester | No | No integration test for SDN zone creation |
+| `Remove-PveSdnZone` | Pester | No | No integration test for SDN zone removal |
+| `New-PveSdnVnet` | Pester | No | No integration test for SDN VNet creation |
+| `Remove-PveSdnVnet` | Pester | No | No integration test for SDN VNet removal |
+| `New-PveStorage` | Pester | No | No integration test for storage creation |
+| `Remove-PveStorage` | Pester | No | No integration test for storage removal |
+| `Invoke-PveStorageDownload` | Pester | No | No integration test for URL download |
+| `Suspend-PveVm` | Pester | No | No integration test for VM suspend |
+| `Resume-PveVm` | Pester | No | No integration test for VM resume |
+| `Move-PveVm` | Pester | No | No integration test (requires multi-node) |
+| `Resize-PveVmDisk` | Pester | No | No integration test for disk resize |
+| `Copy-PveContainer` | Pester | No | No integration test for container clone |
+| `Set-PveCloudInitConfig` | Pester | No | No integration test for cloud-init set |
+| `Invoke-PveCloudInitRegenerate` | Pester | No | Skipped in CI due to bug |
+| `Set-PvePermission` | Pester | Partial | Only tests add, not remove |
+
+**Note**: Many of the gaps above are for operations that would require complex test infrastructure (multi-node clusters, specific storage backends, etc.) and are reasonable omissions for a pre-1.0 module.
+
+---
+
+## Phase 5 -- Security Review
+
+### 5.1 Credential Handling
+
+| Aspect | Status | Details |
+|---|---|---|
+| PSCredential usage | **Good** | `Connect-PveServer` accepts `PSCredential` for password auth |
+| Password extraction | **Acceptable** | `NetworkCredential.Password` is used only at the point of API call |
+| API token in memory | **Acceptable** | Stored as plain `string` in `PveSession.ApiToken` -- standard for PowerShell modules |
+| Credential logging | **Good** | No `WriteVerbose`/`WriteDebug` calls that include credentials |
+| Ticket storage | **Good** | Tickets are stored in `PveSession` only, not written to disk |
+
+**Finding**: The password is extracted from `PSCredential` via `NetworkCredential.Password` which returns a plain `string`. This is the standard pattern for PowerShell binary modules since the PVE API requires the password as a form field. No improvement needed.
+
+### 5.2 TLS/HTTPS
+
+| Aspect | Status | Details |
+|---|---|---|
+| HTTPS enforced | **Good** | `BaseUrl` always constructs `https://` URLs |
+| SkipCertificateCheck | **Good** | Explicit opt-in parameter on `Connect-PveServer` |
+| Certificate bypass | **Acceptable** | Uses `ServerCertificateCustomValidationCallback = ... => true` when opted in |
+| TLS version | **Good** | Left to OS/runtime negotiation (no pinning) |
+| Warning on skip | **Gap** | No `WriteWarning` when `-SkipCertificateCheck` is used |
+
+### 5.3 Input Validation
+
+| Aspect | Status | Details |
+|---|---|---|
+| URL construction | **Good** | Resource paths use string interpolation with validated parameters |
+| Path traversal | **Low risk** | Node names, VM IDs, and storage names are used in URL paths but come from API responses or validated parameters |
+| VMID validation | **Gap** | VmId is `int` type (natural bounds) but no `[ValidateRange]` to enforce PVE's 100-999999999 range |
+| Username format | **Good** | `PveAuthenticator` validates `user@realm` format |
+| API token format | **Good** | Regex validation in `PveAuthenticator.AuthenticateWithApiToken` |
+
+### 5.4 Dependency Security
+
+| Package | Version | Pinned | Notes |
+|---|---|---|---|
+| `Newtonsoft.Json` | 13.0.3 | Yes | Latest stable, no known CVEs |
+| `System.Text.Json` | 8.0.5 | Yes | Latest 8.x patch |
+| `PowerShellStandard.Library` | 5.1.1 | Yes | PrivateAssets=all |
+| `System.Management.Automation` | 7.4.0 | Yes | PrivateAssets=all |
+| `Microsoft.NET.Test.Sdk` | 17.9.0 | Yes | Test only |
+| `xunit` | 2.7.0 | Yes | Test only |
+| `Moq` | 4.20.70 | Yes | Test only |
+| `coverlet.collector` | 6.0.1 | Yes | Test only |
+
+All dependencies are pinned to exact versions. No floating version ranges detected.
+
+### 5.5 Secret Scanning
+
+| Finding | Severity | Location |
+|---|---|---|
+| Hardcoded test password | **Medium** | `integration-tests.yml:42` -- `PVE_PASSWORD: "Testpass123!"` |
+
+The test password is for a throwaway nested PVE instance provisioned and destroyed during CI. It is masked in CI logs via `::add-mask::`. This is acceptable for CI test infrastructure but should be documented.
+
+No other hardcoded credentials, tokens, or sensitive IPs found in source code.
+
+---
+
+## Phase 6 -- PSGallery Publication Readiness
+
+### 6.1 Module Manifest Assessment
+
+| Field | Status | Value |
+|---|---|---|
+| `RootModule` | OK | `PSProxmoxVE.dll` |
+| `ModuleVersion` | OK | `0.1.0` |
+| `GUID` | OK | `a3f7c2d1-84e5-4b9f-a061-3e2d8c5f1a7b` |
+| `Author` | OK | `goodolclint` |
+| `CompanyName` | OK | `Worklab` |
+| `Copyright` | OK | `(c) 2026 goodolclint. All rights reserved.` |
+| `Description` | OK | Meaningful, not placeholder |
+| `PowerShellVersion` | OK | `5.1` |
+| `CompatiblePSEditions` | OK | `Desktop`, `Core` |
+| `DotNetFrameworkVersion` | OK | `4.8` |
+| `RequiredAssemblies` | OK | Lists Core DLL and Newtonsoft.Json |
+| `FormatsToProcess` | OK | `PSProxmoxVE.format.ps1xml` |
+| `CmdletsToExport` | **Excellent** | Explicit list of all 66 cmdlets |
+| `FunctionsToExport` | OK | Empty (binary module) |
+| `AliasesToExport` | OK | Empty |
+| `VariablesToExport` | OK | Empty |
+| `Prerelease` | OK | `preview` |
+| `Tags` | OK | 8 relevant tags |
+| `LicenseUri` | OK | Points to GitHub LICENSE |
+| `ProjectUri` | OK | Points to GitHub repo |
+| `ReleaseNotes` | **Missing** | Not present in PSData |
+| `IconUri` | **Missing** | Not present in PSData |
+| `RequiredModules` | N/A | None required |
+| `HelpInfoURI` | **Missing** | No online help |
+
+### 6.2 License
+
+- MIT License present at repo root.
+- OSI-approved (PSGallery requirement met).
+- `LicenseUri` in manifest points to it.
+
+### 6.3 README Quality
+
+| Aspect | Status |
+|---|---|
+| Installation instructions | **Present** but says "not PSGallery" -- needs update for publication |
+| Quick-start examples | **Excellent** -- multiple scenarios covered |
+| Authentication guide | **Excellent** -- both ticket and API token documented |
+| Multi-cluster usage | **Excellent** -- `-Session` parameter documented |
+| Cmdlet reference table | **Excellent** -- all cmdlets listed with descriptions |
+| Known limitations | **Good** -- v1 limitations documented |
+| Contributing section | **Present** -- basic development setup |
+| Badges | **Missing** -- no build status, PSGallery version, or license badges |
+
+### 6.4 Build & Publish Pipeline
+
+- **Build workflow**: Builds and tests on net48 (Windows) and net9.0 (Windows + Ubuntu).
+- **Unit test workflow**: Tests Pester scripts across PS 5.1/7.5 on Windows/Ubuntu/macOS.
+- **Integration test workflow**: Full tests against live PVE 8 and PVE 9.
+- **PSGallery publish workflow**: **MISSING** -- no workflow for `Publish-Module` or release creation.
+- **GitHub Releases**: Not configured (no release workflow, no tagged releases in git history).
+
+### 6.5 Versioning
+
+- CHANGELOG follows Keep a Changelog format with Conventional Commits.
+- Only an `[Unreleased]` section exists -- no versioned releases yet.
+- Commit messages follow conventional commits (`feat:`, `fix:`, `test:`, `ci:`, etc.).
+
+---
+
+## Phase 7 -- Community & Repo Maintenance Standards
+
+### Files Present
+
+| Item | Status |
+|---|---|
+| `README.md` | Present, comprehensive |
+| `LICENSE` | Present (MIT) |
+| `CHANGELOG.md` | Present |
+| `.editorconfig` | Present |
+| `.gitignore` | Present |
+
+### Files Missing
+
+| Item | Recommendation |
+|---|---|
+| `CONTRIBUTING.md` | Create with dev setup, coding standards, test instructions, PR process |
+| `CODE_OF_CONDUCT.md` | Adopt Contributor Covenant |
+| `SECURITY.md` | Create with vulnerability disclosure policy |
+| `.gitattributes` | Create for line ending normalization (`* text=auto`) |
+| `.github/ISSUE_TEMPLATE/` | Create bug report and feature request templates |
+| `.github/pull_request_template.md` | Create with checklist |
+
+### Commit Conventions
+
+- Conventional Commits used consistently.
+- Scopes include: `ci`, `test`, `fix`, `feat`, `refactor`, `chore`, `docs`.
+- Recent history shows disciplined commit hygiene.
+
+### Branch Protection
+
+Recommended settings (cannot verify via API):
+- Default branch: `main` (confirmed)
+- Require PR reviews before merge
+- Require status checks to pass (build + unit tests + integration tests)
+- Require linear history or squash merging
+
+---
+
+## Phase 8 -- Prioritized Recommendations
+
+### Critical (blocks PSGallery publication or is a security risk)
+
+| # | What | Where | Why | Fix |
+|---|---|---|---|---|
+| C1 | **No PSGallery publish workflow** | `.github/workflows/` | Cannot automate releases | Create `publish.yml` triggered on tag push that runs `Publish-Module` with PSGallery API key from secrets |
+| C2 | **README says "not PSGallery"** | `README.md:30` | Contradicts publication goal | Update installation section to `Install-Module PSProxmoxVE` |
+| C3 | **No ReleaseNotes in manifest** | `PSProxmoxVE.psd1:168-195` | PSGallery strongly recommends release notes | Add `ReleaseNotes` to `PSData` section |
+| ~~C4~~ | ~~Pipeline processing bug~~ | *Verified: all cmdlets use `ProcessRecord`* | N/A -- pipeline processing is correct | No action needed |
+
+### High (significantly impacts quality or community adoption)
+
+| # | What | Where | Why | Fix |
+|---|---|---|---|---|
+| H1 | **No `HelpMessage` on parameters** | All cmdlet files | `Get-Help` shows no parameter descriptions | Add `HelpMessage = "..."` to all `[Parameter]` attributes |
+| H2 | **No `WriteVerbose` in cmdlets** | All cmdlets except Connection | Users cannot debug API calls | Add `WriteVerbose($"Calling {method} {resource}")` before API calls |
+| H3 | **Missing `CONTRIBUTING.md`** | Repo root | Barrier to community contributions | Create with dev setup, coding standards, PR process |
+| H4 | **Missing `SECURITY.md`** | Repo root | No vulnerability disclosure process | Create with responsible disclosure instructions |
+| H5 | **No warning on `-SkipCertificateCheck`** | `ConnectPveServerCmdlet.cs` | Users may not realize they're disabling TLS verification | Add `WriteWarning("Certificate validation is disabled...")` |
+| H6 | **`Reset-PveVm` uses string literal** | `ResetPveVmCmdlet.cs:16` | Inconsistent with all other cmdlets using verb class constants | Change `"Reset"` to `VerbsCommon.Reset` |
+| ~~H7~~ | ~~No `WriteProgress` in `Wait-PveTask`~~ | *Verified: already implemented* | N/A -- `WaitPveTaskCmdlet` already uses `WriteProgress` with percentage and timeout tracking | No action needed |
+| H8 | **Missing GitHub issue/PR templates** | `.github/` | No structured issue reporting | Create bug report, feature request, and PR templates |
+
+### Medium (best practice gaps, test coverage holes)
+
+| # | What | Where | Why | Fix |
+|---|---|---|---|---|
+| M1 | **`Stop-PveVm`/`Reset-PveVm` lack `ConfirmImpact.High`** | `StopPveVmCmdlet.cs`, `ResetPveVmCmdlet.cs` | These can cause data loss | Add `ConfirmImpact = ConfirmImpact.High` |
+| M2 | **No `[ValidateRange]` on VmId** | All cmdlets with VmId parameter | PVE requires VMID 100-999999999 | Add `[ValidateRange(100, 999999999)]` to all VmId parameters |
+| M3 | **Container snapshots not supported** | Services/Cmdlets | Containers support snapshots in PVE but module only handles VM snapshots | Add `Get/New/Remove/Restore-PveContainerSnapshot` cmdlets |
+| M4 | **No integration tests for network modifications** | Integration tests | Network CRUD is tested only at Pester level | Add integration tests for network create/modify/delete |
+| M5 | **No `CODE_OF_CONDUCT.md`** | Repo root | Expected for open-source projects | Add Contributor Covenant |
+| M6 | **No `.gitattributes`** | Repo root | Line ending consistency across platforms | Add `* text=auto` and specific overrides |
+| M7 | **CS1591 suppressed in Core project** | `PSProxmoxVE.Core.csproj:10` | Public API lacks XML documentation | Remove NoWarn and add XML docs to public members |
+| M8 | **No badges in README** | `README.md` | Reduces discoverability and trust | Add build status, PSGallery version, license badges |
+| M9 | **CHANGELOG has only [Unreleased]** | `CHANGELOG.md` | No release history | Create first versioned release entry |
+| M10 | **`Invoke-PveVmGuestExec` lacks ShouldProcess** | `InvokePveVmGuestExecCmdlet.cs:14` | Executes commands inside a VM -- a mutating operation | Add `SupportsShouldProcess = true` |
+
+### Low / Nice-to-have (polish, discoverability, stretch goals)
+
+| # | What | Where | Why | Fix |
+|---|---|---|---|---|
+| L1 | **No `IconUri` in manifest** | `PSProxmoxVE.psd1` | Improves PSGallery listing appearance | Create a module icon and host it, add URI |
+| L2 | **No online help (MAML/platyPS)** | Module | `Get-Help -Online` doesn't work | Generate MAML help using platyPS |
+| L3 | **No firewall cmdlets** | Module | High-value gap for security automation | Implement firewall rule CRUD for cluster/node/VM |
+| L4 | **No backup/vzdump cmdlets** | Module | High-value gap for DR automation | Implement vzdump and backup schedule management |
+| L5 | **No pool management** | Module | Useful for multi-tenant environments | Implement pool CRUD |
+| L6 | **`new Random()` for boundary generation** | `PveHttpClient.cs:393` | Not cryptographically secure (not a security risk for multipart boundaries, but could use `RandomNumberGenerator`) | Consider `RandomNumberGenerator` for .NET 6+ |
+| L7 | **No alias support** | Module manifest | Some users prefer short aliases | Consider adding aliases like `cpve` for `Connect-PveServer` |
+| L8 | **Tagged releases in GitHub** | Repository | No GitHub Releases | Set up release workflow with auto-generated release notes |
+| L9 | **Container migration cmdlet** | Module | VMs have `Move-PveVm` but containers have no equivalent | Add `Move-PveContainer` |
+| L10 | **SDN subnet management** | Module | SDN zones/vnets exist but subnets are missing | Add SDN subnet CRUD |
+
+---
+
+## Summary Statistics
+
+| Metric | Count |
+|---|---|
+| Total cmdlets | 66 |
+| Cmdlets with ShouldProcess | 43 (all destructive/mutating) |
+| Cmdlets with OutputType | 66 (100%) |
+| xUnit test files | 13 |
+| Pester test files | 26 |
+| Integration test contexts | 13 |
+| PVE API areas covered | 10 of ~20 major areas |
+| Critical issues | 4 |
+| High issues | 8 |
+| Medium issues | 10 |
+| Low issues | 10 |
+| NuGet dependencies (runtime) | 2 (Newtonsoft.Json, System.Text.Json) |
+| Security vulnerabilities found | 0 |
+| Hardcoded secrets | 1 (test password in CI, masked) |