Review finding on #97: `statusReached` was sticky, so a guest that reached the
expected status once and then drifted away from it would still be reported as a
success at the deadline, contradicting the fallback's own meaning.
Track the most recent observation instead. A guest that drifts away and never
re-matches now raises PveTaskTimeoutException as it should. A failed poll leaves
the previous observation standing, so an API blip is not read as divergence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PVE publishes a guest's new status while the operation still holds
/var/lock/qemu-server/lock-<vmid>.conf, so WaitForStatusTransition could return
while the guest was still locked and the caller's next request would fail with
"can't lock file ... got timeout".
Integration run 183 failed four tests from this one cause: Restart-PveVm -Wait
returned after 4.1s having seen "running", the following Stop-PveVm spent exactly
10.0s failing to take the lock, and that cascaded into the template convert,
clone, and remove tests. Run 184 - same commit, re-run - passed because its status
poll happened to take 10.1s, by which point the lock had cleared. The same
settling happens either way; the only variable is whether the wait absorbs it or
the next caller does.
The check goes in WaitForStatusTransition because all nine lifecycle call sites
(Start/Stop/Restart/Reset/Resume across VMs and containers) route through it.
`lock` comes from the status/current response the poll already fetches - present
on both qemu and lxc since PVE 5.4, below the module's 7.0 floor - so it costs no
extra request.
If the status is reached but the lock outlasts -Timeout the cmdlet still returns
success, so a call that succeeded before this change cannot become an exception
after it.
Recorded as D015, the guest-lock sibling of D014.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Polls GET /cluster/status for the cluster entry with quorate = 1, bounded
by a timeout (60 s default) and tolerating PveApiException while pmxcfs
and corosync restart during cluster formation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PVE's cluster-create task completes before corosync converges. Until the
node is quorate it rejects a join with "cluster not ready - no quorum?",
so New-PveCluster -Wait followed by Add-PveClusterMember failed for every
caller. Wait for quorum after the task, bounded by -Timeout (default 60 s,
following the -Wait timeout convention used by Stop-PveContainer and
Reset-PveVm). See DECISIONS.md D014.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Minor bump: this release adds features (New-PveVm disk controller/IO
options and Get-PveVmConfig key surfacing, #65) alongside two bug fixes
(#64 semicolon form-encoding, #68 guest-exec argv).
Updates the three release artifacts in lockstep: psd1 ModuleVersion,
psd1 ReleaseNotes, and CHANGELOG ([0.2.0] cut from [Unreleased]).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- VmService.ExecuteGuestCommand: guard against null elements in -Args.
The old JSON-serialization tolerated nulls (as "null"); the repeated-key
path would NRE in EncodeFormValue. Throw a clear ArgumentException instead.
- findings.json: refresh the stale counters block (untouched since F085) to
the actual ledger state — next_id 92, resolved 83 — and bump last_updated
to 2026-05-22. last_scan_date stays 2026-03-26 (F086–F091 came from issue
triage, not a formal review scan).
- VmServiceTests: add empty-array (single command entry) and null-element
(throws) cases.
Note: F091 is the correct next ID — F086–F090 already exist from prior
merged PRs (#60/#61/#66/#67); only the counters were lagging.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ExecuteGuestCommand JSON-serialized the args array into the agent/exec
'input-data' field — which is the process's STDIN, not its arguments. So
guest commands ran with no argv: cmd.exe started interactively and the
JSON blob ['/c','echo',...] arrived at its prompt.
PVE's agent/exec 'command' parameter is itself an array (element 0 = the
executable, the rest = argv) sent as repeated form keys. The low-level
client couldn't express repeated keys (Dictionary<string,string> only),
so:
- Add PostAsync(string, IEnumerable<KeyValuePair<string,string>>) to
IPveHttpClient/PveHttpClient; BuildFormContent now emits one key=value
field per pair, so a key may repeat.
- ExecuteGuestCommand builds command = [exe] + args as repeated 'command'
fields and no longer touches input-data.
Tests: form-encoder repeated-key + per-value encoding cases; VmService
tests asserting the command array, order, and absence of input-data; an
integration regression guard that echoes an arg and checks it round-trips
as stdout.
Tracked as F091. Closes#68.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that -ScsiHardware is part of HasDiskOptions(), the old "options were
ignored" wording was misleading: scsihw is written unconditionally as a
VM-level key and is never ignored. Reword to state that the per-disk
options are the ones dropped, and that -ScsiHardware (if specified) is
still applied. Accurate whether or not -ScsiHardware was passed.
Addresses PR #67 follow-up review.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- HasDiskOptions(): include -ScsiHardware so the "disk options ignored"
warning fires when -ScsiHardware is passed without -DiskStorage/-DiskSize.
- PveVmConfig.AdditionalProperties: lazy-init a backing field so the native
dictionary is built once rather than reallocated on every property access
(matters when iterating many configs in a pipeline). Safe because the model
is effectively immutable after deserialization.
- New-PveVm.Tests.ps1: add a case asserting -DiskIoThread on scsi with a
wrong -ScsiHardware (virtio-scsi-pci) is rejected, covering the validator's
"!= virtio-scsi-single" branch (not just the null case).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New-PveVm (F089):
- Add -DiskBus (virtio/scsi/sata/ide, default virtio), -ScsiHardware (scsihw),
-DiskIoThread, -DiskAio, -DiskSsd, -DiskDiscard, -DiskCache so a tuned disk
(e.g. virtio-scsi-single + scsi0,iothread=1,aio=native,ssd=1,discard=on) can
be created in one call instead of diskless + a hand-built Set-PveVmConfig string.
- Disk spec built via BuildDiskSpec; ValidateDiskOptions runs before ShouldProcess
and rejects ssd on virtio and iothread on sata/ide or scsi-without-virtio-scsi-single
with clear errors, instead of letting PVE fail at VM start.
Get-PveVmConfig (F090):
- PveVmConfig was a fixed allow-list, silently dropping keys like scsihw, efidisk0,
tpmstate0, hostpci0. Add typed scsihw/efidisk0/tpmstate0 plus a [JsonExtensionData]
catch-all exposed as AdditionalProperties (native types via JsonHelper.ToNative,
per D013 — no JToken leakage). Makes the disk tuning above verifiable by reading
the config back.
Closes#65.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PveHttpClient.EncodeFormValue encoded &, =, +, space, and % but left ';'
literal. PVE's application/x-www-form-urlencoded parser treats a raw ';'
as a field separator (the historical alternative to '&'), so a value like
boot=order=scsi0;ide2
was split into 'boot=order=scsi0' plus an empty 'ide2' field, and PVE
rejected the PUT with "ide2: unable to parse drive options". This broke
any multi-device boot order set via Set-PveVmConfig -AdditionalConfig,
and any other value containing ';'.
Encode ';' as %3B. Safe under the existing minimal-encoding policy that
keeps ':' and '!' literal for cluster-join: cluster-join payloads never
contain ';', and PVE url-decodes config form values.
Tracked as F088. Closes#64.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Doc hygiene catch-up surfaced by the PR #62 review:
- CHANGELOG.md: the [Unreleased] section had accumulated all post-preview
work without ever being cut into release entries. Promote it into:
- [0.1.3] - new fixes from #58 (DiskSize normalization) and #59
(HttpClient -TimeoutSeconds + RequestTimeout surfacing)
- [0.1.2] - #43/#44/#45 fixes from PR #46
- [0.1.1] - the cmdlet expansion + OpenAPI validation that
actually shipped to PSGallery as 0.1.1
Reset [Unreleased] to empty.
- src/PSProxmoxVE/PSProxmoxVE.psd1: replace the stale "Initial preview
release" ReleaseNotes (carried over since 0.1.0-preview) with actual
0.1.3 notes. PSGallery shows this on the version page.
- CLAUDE.md: document the release process so future bumps update the
psd1 version, psd1 ReleaseNotes, and CHANGELOG together before the
tag is cut. Prevents this hygiene gap from recurring.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Releases #58 (LVM disk-size unit normalization) and #59 (HttpClient
timeout / -TimeoutSeconds) fixes to PSGallery.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The when filter (ex.InnerException is TimeoutException) only matches on
.NET 5+. On .NET Framework 4.8 — which CI exercises via the test project's
net48 target — HttpClient.Timeout throws a bare TaskCanceledException
with no inner exception, so CI failed:
Expected: typeof(PSProxmoxVE.Core.Exceptions.PveApiException)
Actual: typeof(System.Threading.Tasks.TaskCanceledException)
---- System.Threading.Tasks.TaskCanceledException : A task was canceled.
PveHttpClient.SendAsync never passes a CancellationToken to the inner
HttpClient.SendAsync, so the only way a TaskCanceledException can reach
this catch is HttpClient.Timeout firing — true on net48, .NET Core, and
.NET 5+. Drop the filter and wrap unconditionally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When HttpClient.Timeout elapses, .NET throws TaskCanceledException — not
HttpRequestException — so the existing catch in PveHttpClient.SendAsync
missed it and callers got a raw stack trace. With -TimeoutSeconds now
configurable and documented, this gap became user-visible.
In .NET 5+ HttpClient surfaces transport timeouts as TaskCanceledException
with a TimeoutException inner; user-driven token cancellation does not.
Catch by that inner-type signature and rethrow as PveApiException with
HttpStatusCode.RequestTimeout, the resource path, and a message that
reports the configured timeout.
Adds SendAsync_TimeoutFires_ThrowsPveApiExceptionWithRequestTimeout which
swaps in a delaying HttpMessageHandler with a 50ms timeout to exercise
the path deterministically. Drops the redundant
DefaultSessionTimeoutIs100Seconds test (covered by
PveSessionTests.Timeout_DefaultIs100Seconds and the existing flow-through
test).
Addresses PR #61 review feedback.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves findings.json conflict — F086 (from #60, merged into main) and
F087 (this branch) both append to the trailing findings array. Kept both
entries, in numeric order.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PveHttpClient was constructed without setting HttpClient.Timeout, so
.NET's 100s default applied to every request. Multi-GB ISO uploads via
Send-PveFile on a real LAN reliably tripped this with TaskCanceledException
after 100 seconds, and there was no way to override it.
- PveSession gains a Timeout (TimeSpan) property, defaulting to 100s.
- PveHttpClient accepts an optional per-instance timeout override that
takes precedence over the session timeout.
- Connect-PveServer exposes -TimeoutSeconds to set the session default.
- Send-PveFile and Invoke-PveStorageDownload expose -TimeoutSeconds with
a 30-minute implicit default so large uploads/downloads do not trip
the 100s default. -TimeoutSeconds 0 means Timeout.InfiniteTimeSpan.
Tracked as F087. Closes#59.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- SizeParser: wrap TB-suffix overflow in try/catch so callers get
ArgumentException with the parameter name rather than OverflowException.
- New-PveVm/New-PveContainer: validate -DiskSize/-RootFsSize before
ShouldProcess so typos like "512M" are rejected even with -WhatIf and
even when the matching -DiskStorage/-RootFsStorage is omitted.
- Add Pester tests for the new DiskSize and RootFsSize validation paths,
including a new New-PveContainer.Tests.ps1.
- Add SizeParserTests coverage for the TB overflow path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Disk and rootfs size strings were interpolated directly into the disk
spec as "<storage>:<size>", so "60G" produced "local-lvm:60G". On
LVM/LVM-thin storages PVE parses the value after the colon as a volume
name unless it is a bare integer, returning "unable to parse lvm volume
name '60G'". File-backed storages mask this by accepting either form.
SizeParser.NormalizeToGibibytes() now strips G/GB/T/TB suffixes and
returns a bare GiB integer string, so the documented "32G" call shape
works on every storage type. Sub-GB units are rejected with a clear
error rather than being silently truncated.
Tracked as F086. Closes#58.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add 70 xUnit tests that validate every ValidateSet in the module
against the PVE OpenAPI spec. Three bugs found and fixed:
- Storage: remove `glusterfs` (dropped in PVE 9), add `btrfs`, `esxi`
- Backup compression: `none` → `0` (PVE uses "0" not "none")
- Cluster resources: remove `lxc` filter (PVE uses `vm` for both)
The pve-api-enums.json fixture (199KB) is extracted from the full
OpenAPI spec and contains parameter enum values for 302 API paths.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PVE 9 API requires 'type' on PUT /cluster/ha/rules/{rule} even for
updates. Added mandatory Type parameter with ValidateSet for
node-affinity and resource-affinity.
Updated integration test and Pester unit tests to pass -Type.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
.NET's FormUrlEncodedContent over-encodes characters like : and !
(%3A, %21) in form values. PVE's internal API consumers (specifically
the cluster join process) do not URL-decode these values before using
them, causing fingerprint comparison failures and password mismatches.
Replace with BuildFormContent/EncodeFormValue that only encodes
characters that break form parsing (&, =, +, space, %). This matches
curl's -d behavior and fixes cluster join "Cluster join aborted!" errors.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract re-auth retry into ReauthenticateWithRetry helper (fixes
cognitive complexity warning)
- Retry auth up to 10x with 3s delay — node B's auth services need
time to restart after joining the cluster
- Wrap join in try/finally so the test always reconnects to node A,
even if the join fails
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cluster join restarts auth services on the joining node, which
invalidates the PVE ticket mid-poll. When WaitForTask gets a 401,
catch it and re-authenticate with the password already available
in the cmdlet, then retry the wait.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
xUnit (47 new tests, 429 total):
- ClusterConfigServiceTests: 25 tests covering all 14 service methods
including URL encoding, null guards, and error responses
- HaServiceTests: 22 tests covering resources, groups, status, and rules
with URI encoding verification (vm:100 → vm%3A100)
Pester (187 new tests, 1525 total):
- ClusterConfigCmdlets.Tests.ps1: 11 cmdlets tested
- HaCmdlets.Tests.ps1: 14 cmdlets tested
Integration (ClusterConfig.Integration.Tests.ps1):
- Full 2-node cluster lifecycle with -Wait for task completion
- Uses root@pam ticket auth for cluster create/join operations
- HA group tests skip on PVE 9.0+ (groups migrated to rules)
- JArray indexing uses .Item() for PowerShell compatibility
Cmdlet improvements:
- New-PveCluster, Add-PveClusterConfigNode, Add-PveClusterMember now
support -Wait switch to block until task completes (via TaskService)
- GetClusterConfig returns JToken to handle array responses on standalone
- OutputType updated to PveTask for task-returning cmdlets
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RegenerateCloudInitImage was calling GET /cloudinit/dump?type=user
which returns the cloud-init YAML content, not a task UPID. The
cmdlet then passed this YAML string to WaitForTask, causing a 501
error trying to poll a URI like "GET nodes/.../tasks/%23cloud-config..."
Fixed to call PUT /nodes/{node}/qemu/{vmid}/cloudinit which is the
correct regeneration endpoint that returns a UPID.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
D013 compliance scan found 2 remaining Newtonsoft type exposures:
- PveClusterJoinInfo.Nodelist: JArray → List<Dictionary<string, object?>>
- PveClusterJoinInfo.Totem: JObject → Dictionary<string, object?>
Both now use NativeListConverter/NativeDictionaryConverter for
deserialization. Full D013 compliance report added.
Updated findings.json: F085 status open with scan evidence.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace null-forgiving operator on Marshal.PtrToStringUni with
null-coalescing fallback (AddPveClusterMemberCmdlet.cs:64)
- GetNextId now throws InvalidOperationException instead of silently
returning 0 when API response cannot be parsed
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract ParseLinks helper to PveCmdletBase for shared link parsing
with WriteWarning on malformed entries (was duplicated in 3 cmdlets)
- Fix GetClusterConfig to return data payload, not full API envelope
- Fix OutputType on GetPveClusterConfigCmdlet to JObject
- Fix link doc comments to use correct key format (link0..link7)
- Add null-safe Properties hashtable conversion in HA rule cmdlets
- Use case-insensitive Mode comparison in MovePveHaResourceCmdlet
- Remove unused using directives
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
F039 (regressed): Replace bare catch blocks with filtered catches that
exclude OutOfMemoryException and StackOverflowException, per D004.
- VmService.PingGuestAgent: catch now filters fatal exceptions
- ImportPveOvaCmdlet: catch now filters fatal exceptions + WriteVerbose
F084 (new): Add PveSession format view to PSProxmoxVE.format.ps1xml
that shows only Hostname, Port, AuthMode, IsExpired, and ServerVersion
in default table output. Sensitive properties (Ticket, ApiToken,
CsrfToken) are hidden from default display but remain accessible
via Select-Object * or direct property access.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New-PveStorage now supports configuring iSCSI storage backends natively:
-Target: iSCSI target IQN (e.g. iqn.2024-01.com.example:storage)
-Portal: iSCSI portal address (host:port, defaults to server:3260)
Also refactored ProcessRecord to use AddIfNotEmpty helper, reducing
cognitive complexity.
Added unit tests for all iSCSI/NFS parameter metadata and a new
SharedStorage.Tests.ps1 integration test file that tests NFS and iSCSI
storage create/verify/status/delete lifecycle against the Docker-based
storage containers.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- generate-help.ps1: rewritten to use Update-MarkdownHelp (merge) by
default, preserving hand-written descriptions and examples. Only
New-MarkdownHelp on genuinely new cmdlets. -Force flag for full regen.
- docs/cmdlets: parameter metadata updated via platyPS merge, CRLF
normalized to LF
- Removed stale Send-PveIso.md (cmdlet renamed to Send-PveFile)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
goodolclint-claude[bot]