Previous approach (/code-review:code-review --comment) was based on
incorrect documentation research. Reviewing the actual official
examples at anthropics/claude-code-action/examples/pr-review-*.yml
reveals the correct pattern:
1. Use a custom prompt with explicit review instructions (not a
plugin slash command)
2. Use claude_args with --allowedTools to enable the MCP inline
comment tool and gh pr CLI commands — this is what lets Claude
actually post to the PR
3. Enable track_progress: true for visual progress tracking
Without --allowedTools, Claude has no way to post anything because
the tools for PR commenting aren't allowed by default.
Also removed the plugins and plugin_marketplaces inputs since
they're not needed — the review runs via prompt instructions and
the allowed tools alone.
The custom prompt is tailored to PSProxmoxVE with focus areas
specific to the module: DECISIONS.md compliance, cmdlet
conventions, API correctness against the PVE OpenAPI spec,
test coverage, and security.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dependabot PRs were being rejected with:
Workflow initiated by non-human actor: dependabot (type: Bot)
Add allowed_bots: 'dependabot[bot]' to permit dependency update reviews.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace single-version fixture with per-version specs from pve-api.
Tests now validate enum values against PVE 7 (best-effort), 8, and 9.
Key findings from version-specific specs:
- VM.Monitor: valid in PVE 7+8, removed in PVE 9
- VM.Replicate: added in PVE 9 only
- VM.GuestAgent.*: added in PVE 9 only
- Mapping.*: added in PVE 8+
- glusterfs: not in any version (removed before PVE 7)
Co-Authored-By: Claude Opus 4.6 (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>
Publish-Module -Path requires the directory name to match the module
name. Changed artifact download path from ./publish/netstandard2.0/
to ./publish/PSProxmoxVE/ so Publish-Module can find the manifest.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The ValidateSet attribute was intentionally removed from the Type
parameter during merge review to allow future rule types without
code changes. Updated test to only assert Type is mandatory.
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>
- Rename ZZ.ClusterConfig.Integration.Tests.ps1 → 16_Cluster.Tests.ps1
- Replace manual env var setup with _IntegrationHelper.ps1 + Connect-TestPve
- Use credential auth (root@pam) throughout — no API tokens
- Use $script:PasswordB for node B auth (from helper)
- Access JoinInfo.Nodelist as List<Dictionary> (native types from D013)
- Keep cluster-specific skip helpers (Skip-IfNoCluster, Skip-IfPve9HaGroups)
- Remove redundant Connection context (Connect-TestPve handles it)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PVE 9 rejects rules referencing unmanaged resources. Test now creates
a minimal VM, registers it as a disabled HA resource, then creates
the node-affinity rule. Cleanup removes rule, resource, and VM.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PVE 9 rejects 'affinity' as an unexpected property on POST
cluster/ha/rules — it's implied by the node-affinity type.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Full lifecycle: create node-affinity rule, list, get by ID, update
comment, delete, and verify deletion. Uses vm:99999 as a synthetic
resource SID. Tests skip on PVE 8 (rules not available).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cluster tests create a 2-node cluster that cannot be torn down via API
(quorum loss). Prefix with ZZ so they execute after all other
integration tests that assume standalone nodes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removing a node from a 2-node PVE cluster via REST API is not supported
— the remaining node loses quorum mid-operation, causing "no quorum!"
errors. PVE requires stopping corosync on the departing node first
(pvecm expected 1), which is not available via the REST API.
Replace removal tests with a final cluster state verification.
Test infrastructure handles cleanup via reprovisioning.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove-PveClusterConfigNode requires root@pam (not API token).
Fixed both the test context and AfterAll cleanup to connect with
root@pam credentials before attempting node removal.
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>
All cluster operations (create, status, join, options, cleanup) require
root@pam ticket auth — API tokens lack Sys.Audit and Sys.Modify on /.
Changed test to connect as root@pam once during cluster creation and
stay on that session for the entire lifecycle.
Removed redundant Connect-PveServer calls that switched between
root@pam and API token between contexts.
Added 5s stabilization sleep after cluster creation for corosync.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Convert JoinInfo.Nodelist to native PS array via ConvertFrom-Json
(JArray .Item() still hits IEnumerator error in PowerShell)
- Use root@pam for cluster options context (delete requires Sys.Modify)
- Reconnect with API token after options restore
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>
RUNNER_TEMP (/__w/_temp in CI) is container-local and invisible to
the Docker host. Files written there can't be bind-mounted into
sibling containers (answer server, storage). This was the root cause
of the "not a directory" mount failures in CI.
Removed RUNNER_TEMP from the WORK_DIR fallback chain. WORK_DIR now
always defaults to CACHE_DIR/work (/opt/pve-integration/work/) which
is on the shared mount visible to both the CI container and Docker host.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mounting the whole directory to /app overwrites server.py inside the
container, causing "can't open file '/app/server.py'" errors.
Mount the two paths individually instead:
- answer_server_dir/answers → /app/answers
- answer_server_dir/default.toml → /app/default.toml
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GoodOlClint