Commit Graph

20 Commits

Author SHA1 Message Date
goodolclint-claude[bot] 40f4eae9e2 docs: record that changelog entries are batched, and that the up-to-date rule is off (#185)
* docs: record that changelog entries are batched, and that the up-to-date rule is off

The automated reviewer on #182 requested a CHANGELOG.md edit on a fix
PR because the batching convention lived only in the remediation plan.
The reviewer reads CLAUDE.md from main on every run, so the rule goes
here. Also records the branch-protection change from 2026-09-02.

Closes #177

* docs: drop the remediation reference from the branch-protection note

---------

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
2026-09-02 16:58:48 -05:00
goodolclint-claude[bot] 09a2384cd6 docs: record the branch protection that is actually configured (#133)
* docs: record the branch protection that is actually configured

CLAUDE.md has claimed since March that main had 'required build checks,
required review, admin enforced'. Two of those three were wrong: direct pushes
to main were still permitted until 2026-09-02, and admin bypass is now
deliberately available rather than enforced.

Records what is actually set: required checks, required PR with an approving
review, stale reviews dismissed on push, and review from Code Owners required.
Explains the narrow CODEOWNERS, so a future session knows why a workflow or ADR
change needs the operator while ordinary work does not.

Admin bypass is available on purpose. GitHub does not let an author approve
their own PR, so without it an operator-authored governance change would
deadlock. It is not a hole here: App installations do not get admin bypass, so
it does nothing for a compromised or injected bot.

ADR 0025 previously said enabling code-owner review was the operator's action
and not done by that decision. It has been done, so the dismissal gate is now
defence-in-depth rather than the load-bearing control - GitHub enforces the
property directly, including against approvals from claude.yml, the ungated
path the gate cannot see.

* docs: trim a clause ADR 0025 stated twice in four lines

Review nit on #133. The 'claude.yml is an ungated second path the dismissal
step cannot see' clause appeared in consecutive paragraphs. The first states
it; the second now refers back rather than restating.

---------

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
2026-09-02 10:45:26 -05:00
goodolclint-claude[bot] b90791e2bf docs: migrate DECISIONS.md to house-format ADRs, and retire the review folder (#131)
D001-D021 become ADR 0001-0021 in docs/decisions/, one decision per file.
D017's PESTER_VERSION amendment was a second decision in one entry and becomes
ADR 0022. ADR 0023 records the migration and reverses the lane2-change-plan
ruling that deliberately kept DECISIONS.md until the CI lane work landed.

DECISIONS.md is reduced to a stub with a D-to-ADR redirect table, so the four
released CHANGELOG entries and older issue bodies that cite it degrade to a
redirect rather than a dead reference.

docs/review/ and docs/lane2-change-plan.md are deleted (ADR 0024). Of 91
findings, 83 were resolved and six of the seven still open were already GitHub
issues; F021 was the exception and is now #130.

CLAUDE.md's Key Conventions list gains the two rules it was missing and becomes
the checklist, with the ADRs carrying rationale.
2026-09-02 14:49:07 +00:00
goodolclint-claude[bot] fc073e7e2a ci: pin Pester by exact version everywhere it is installed or imported
Pester was installed with -MinimumVersion 5.0 and no ceiling in the CI job
image, both install steps in unit-tests.yml, and both Import-Module calls, plus
the suite's own import inside the container. The image is rebuilt on every CI
run and Pester is installed fresh on every unit-test run, so PSGallery chose the
version — a new major could reach the required PR checks with no commit here,
surfacing as unexplained test breakage on whichever PR ran next.

It had already happened. Steps named "Install Pester 5" were resolving 6.1.0 on
both legs, because Pester 6 declares PowerShellVersion 5.1 and so installs on
Windows PowerShell too. Nothing broke — the suite uses only constructs common to
5 and 6, and runs 1566/0 under 6.1.0 with no deprecation warnings — but nobody
chose it. The step names are corrected; they had been describing an install that
stopped happening some time ago.

Pinning the install alone is not enough, in two ways review found:

An unset variable does not fail. -RequiredVersion accepts an empty value and
degrades to "latest" for Install-Module and to "any" for Import-Module, both
exiting 0, so a renamed or dropped env key would silently restore the float this
commit removes. A guard step now fails the job instead.

The point of use was still floored. run-integration.sh imported the suite's
Pester with -MinimumVersion 5.0, so a second Pester reaching PSModulePath would
win regardless of what was installed. The Dockerfile now promotes the ARG to ENV
so the version is discoverable at runtime, and that import is pinned to it.

The pin lives in two files, so shell-selfchecks asserts they agree — split-brain
between the workflow and the image is precisely the unexplained breakage this is
meant to prevent. CONTRIBUTING.md and CLAUDE.md are updated too; the contributor
instructions were a third floating install site.

Recorded as an amendment to D017 — the same principle as the nested PVE package
pin, applied to the lane's own tooling.
2026-09-01 18:08:02 -05:00
goodolclint-claude[bot] 298df2a30b docs: cite the right lines for the fixed VMIDs
The review caught that run-integration.sh:106-138 covers pve_vmid() but not
STORAGE_VMID, which is set at line 80. Cite both accurately.
2026-09-01 17:42:07 -05:00
goodolclint-claude[bot] d5678284bc docs: full macOS recipe for the integration flow, and the shared VMID hazard
Expands the Rosetta note into a working recipe, after running the whole
provision -> test -> cleanup lifecycle on an Apple Silicon Mac against the real
parent cluster.

Compose is the wrong entry point on a Mac: its dev-infra service builds rather
than pulls, and bind-mounts /opt/pve-integration, which does not exist there.
The macOS path pulls the image CI already built and drives run-integration.sh
with docker run. Records that GHCR needs a classic PAT, since fine-grained
tokens cannot reach it at all and the failure gives no hint why.

Restores the x86 compose instructions, which the previous commit's rewrite
consumed, and hoists the fixed-VMID warning out of the macOS section — 5080,
5091 and 5092 are shared with CI on the same parent cluster whatever host you
run from, so a local run during a CI run collides, and a skipped force-cleanup
fails the next run's headroom guard.

Also warns that emulation runs the suite ~40% slower and so loses the
qemu-server flock race (#113) that CI wins: Reset-PveVm, clone and
Set-PveVmConfig fail locally on a tree that is green in CI. Provisioning and
cleanup are unaffected.
2026-09-01 17:36:26 -05:00
goodolclint-claude[bot] 439a691516 docs: the dev-infra image needs Rosetta on Apple Silicon
"x86 only" was too strong. The image is amd64-only — proxmox-auto-install-assistant
and the HashiCorp apt repo publish no arm64 — but it builds and runs on Apple Silicon
once Docker Desktop's Rosetta emulation is on.

Under the default qemu translation pwsh starts and reports its version, then segfaults
on module discovery (uncaught target signal 11), which fails the build at
Install-Module Pester and would fail Pester at test time. With Rosetta enabled the same
Dockerfile builds to within 150 bytes of the image CI pushed for this commit, and
Invoke-Pester runs.

Worth stating explicitly because the failure is silent: the build step exits 1 with no
diagnostic output, which reads as a Dockerfile defect rather than an emulation problem.
2026-09-01 16:53:54 -05:00
goodolclint-claude[bot] 08ee3ae249 chore: repair the local dev path and delete its dead scaffolding
The local dev environment had drifted badly from CI. Remove the parts that no
longer describe anything real, and make the rest match how CI actually runs.

Delete tests/dev.ps1. It wrapped run-integration.sh, which CI calls directly,
and duplicated the module build that script already performs internally. As a
second entry point it drifted: it still offered the PVE 8 leg retired in #88,
mounted the Docker socket for storage containers replaced by the storage VM in
#87, and pointed its remote-host examples at a runner decommissioned in the ARC
migration. All four documents describing it used a positional syntax that bound
the bare word to -Tests and then fell through to -Shell, so every documented
command silently opened a container shell. Recorded as D019.

Delete tests/infrastructure/runner/, a self-hosted-runner-in-Docker superseded
by Actions Runner Controller.

Make disk_storage and iso_storage required. Their defaults named a NAS that the
lab replaced with Ceph, and CI overrides both from repository variables, so the
defaults only ever misled local runs. require_env now fails at the top of a run
rather than at terraform apply, and the descriptions point at tests/.env.test
because cmd_provision deletes terraform.tfvars before applying.

preflight-cleanup.sh no longer falls back to the literal "local" storage. An
unset TF_VAR_iso_storage now skips only the ISO branch, leaving VM destroy and
state cleanup intact, and emits a workflow annotation: force-cleanup is the
only cleanup CI runs and it wipes Terraform state, so a silent skip strands the
uploaded ISO with nothing left to reclaim it.

Drop docker-ce-cli and the /var/run/docker.sock mount. Nothing in the container
has called docker since #87 moved storage into a VM; the remaining docker calls
run inside that VM over SSH. The CI job image is built from the same target, so
this also removes a third-party apt repository from its supply chain.

Rewrite tests/.env.test.example against what the code now requires, and fix the
documented commands in CLAUDE.md, README.md, copilot-instructions.md and the
integration README.
2026-09-01 16:27:10 -05:00
goodolclint-claude[bot] 561c55ec57 docs: review before pushing, not after 2026-09-01 19:29:38 +00:00
goodolclint-claude[bot] 8f9f546b7a docs: env block takes effect immediately, not at next session start 2026-09-01 13:45:13 -05:00
goodolclint-claude[bot] 89a5a34337 docs: prefer MCP push_files; git push is the unverified large-push fallback
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 13:43:55 -05:00
goodolclint-claude[bot] c706315483 chore: attribute agent commits to the bot identity
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 13:42:10 -05:00
Clint Branham 79c97ec211 docs: cut CHANGELOG into versioned entries, refresh ReleaseNotes
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>
2026-05-20 19:23:29 -05:00
Clint Branham 197e206123 chore: scan-9 review findings + fix publish secret name
- Resolve F039 (bare catch regression), F053 (HA coverage), F060
  (cluster config coverage), F084 (PveSession secret hiding)
- Update findings.json: 11 open → 7 open, 0 regressions
- Update REVIEW_REPORT.md with scan-9 results
- Fix publish.yml to use NUGET_API_KEY secret (matches other projects)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-26 08:58:35 -05:00
Clint Branham f311c738b4 docs: review scan-8 findings update
Scan-8 full re-scan of PSProxmoxVE module (2026-03-24).

New findings:
- F084 [medium/security]: PveSession exposes auth secrets in pipeline output

Regressed:
- F039 [medium]: Bare catch blocks in VmService.PingGuestAgent and
  ImportPveOvaCmdlet (D004 violation)

Resolved:
- F059: VM/CT-level firewall was already supported via Level parameter

Findings DB: F001-F084, 10 open + 1 regressed + 72 resolved + 1 wont_fix

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 18:15:46 -05:00
Clint Branham f21c7f84b7 feat(ci): unify CI and local integration test infrastructure
- Replace tests/infrastructure/Dockerfile with tests/Dockerfile.test
  (single multi-stage Dockerfile for both CI and local dev)
- CI container-image job now builds from Dockerfile.test target dev-infra
- Add ARM support: PowerShell installed via dotnet tool on arm64,
  APT package on amd64
- Replace tests/dev.sh (bash) with tests/dev.ps1 (PowerShell) for
  cross-platform support (Windows, macOS, Linux)
- Add -DockerHost parameter for running x86 containers on a remote
  Docker host from ARM Macs (rsyncs repo, uses SSH Docker transport)
- Add -NoCleanup switch to keep nested PVE VMs after integration tests
- integration command now provisions nested PVE VMs instead of testing
  against a pre-existing PVE directly
- Share /opt/pve-isos host path between CI and local dev (was separate
  Docker named volume)
- Delete tools/Invoke-Tests.ps1 (unused, overlapped with run-integration.sh)
- Add .gitignore entries for Terraform state/artifacts
- Update all documentation references

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 10:41:28 -05:00
Clint Branham ee717cccd1 docs: update CLAUDE.md with PR workflow and dev container instructions
All changes now go through pull requests. Branch protection is enabled
on main (required build checks, required review, admin enforced).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 09:21:59 -05:00
Clint Branham b4754c72a0 docs: update README, CHANGELOG, SECURITY.md, and review artifacts
- README: move Set-PveStorage to Storage section, Set-PveApiToken to
  Access section (were miscategorized under "Access — Groups & Domains")
- CHANGELOG: add Changed section with 13 architectural improvements,
  expand Fixed section with all ConfirmImpact and credential fixes
- SECURITY.md: add SecureString detail, Input Validation section,
  Dependabot mention, SkipCertificateCheck warning note
- CLAUDE.md: update review system reference to scan-7
- findings.json: F050 resolved (URL encoding), F031 resolved (credentials)
- REVIEW_REPORT.md: scan-7 report

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 17:41:19 -05:00
Clint Branham 89bead7a0e fix: add -Confirm:$false to Restart-PveContainer integration test (F080, F081)
Restart-PveContainer gained ConfirmImpact.High in the F062 fix, which
correctly prompts for confirmation on destructive operations. The
integration test was missing -Confirm:$false, causing a
NullReferenceException in non-interactive CI. The cascading failure also
broke the Copy-PveContainer test (F081).

Also updates CONTRIBUTING.md to reference .NET SDK 10.0+ (was 9.0+) to
match all CI workflows and test project TFM (F083).

Includes scan-6 review report and findings database update (F001-F083).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 14:48:11 -05:00
Clint Branham 0fa92d689f docs(review): scan-4 review report and findings update
Add scan-4 full review: 1 resolved (F040), 6 new findings (F071-F076).
27 open findings, 49 resolved. No regressions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 09:07:51 -05:00