docs: a PVE-behaviour claim documented against the published spec is verified, not deferred (ADR 0026) (#170)

* docs: a PVE-behaviour claim the PR documents against the spec is verified, not deferred The reviewer fetches the cited proxmox_api permalink with gh api, names it in the review, and approves when the spec supports the claim. Only inferred claims still go to the operator. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs: record ADR 0026 and tighten the spec-verified rule per review Commit-anchored permalinks only, the gh api invocation spelled out, and the trade-off against the server-behaviour caveat stated in the prompt and in the ADR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs: cite the per-version OpenAPI file, and attribute the reserved list to the prompt Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-03 18:55:33 +00:00 · 2026-09-02 19:02:30 +00:00
parent 147c2c6007
commit 8ec09c2b84
2 changed files with 64 additions and 2 deletions
@@ -46,7 +46,23 @@ PR's checkout.
   OpenAPI spec — see `tests/PSProxmoxVE.Core.Tests/Fixtures/pve-api-enums.pve*.json`
   for valid values per PVE version. A value the schema accepts is not
   necessarily a value PVE acts on; where a PR claims server behaviour, ask
-   whether it was observed or inferred.
+   whether it was observed, documented, or inferred. Documented means the PR
+   cites a commit-anchored permalink into https://github.com/GoodOlClint/Proxmox_API
+   (`.../blob/<sha>/pve/...`, never a branch reference) to the OpenAPI spec file
+   for the PVE version (`pve/openapi/pve-openapi.pve<N>.json`, one file per
+   version; a `#L` range points at the endpoint), or to the `pve/CHANGELOG.md`
+   entry for return-field history. Fetch it with
+
+       gh api -H "Accept: application/vnd.github.raw+json" \
+         "repos/GoodOlClint/Proxmox_API/contents/<path>?ref=<sha>"
+
+   and read it. A claim the spec supports counts as verified; name the permalink
+   you checked in the review. The spec is the published contract, not the
+   server: it can lag or differ, and the wave-end integration run is what
+   catches that. Naming the permalink is what lets a later live failure be
+   traced to the spec and the server disagreeing, rather than to a guess. A
+   cited link that is not commit-anchored, does not support the claim, or
+   cannot be fetched leaves the claim inferred. See ADR 0026.

 4. **Tests.** New cmdlets should have xUnit service tests and Pester
   parameter-validation tests. When you criticise a test, say what behaviour it
@@ -101,7 +117,10 @@ sign-off.
  values — a PR that edits them can make a wrong value look spec-compliant to
  you;
 - changes branch protection, publishing, or release tagging;
- claims live PVE behaviour that CI does not exercise.
+- claims live PVE behaviour that CI does not exercise and the PR does not
+  document against the spec (see API correctness above). A spec-verified claim
+  does not need the operator; an inferred one does. This is the trade-off
+  ADR 0026 records.

 ## Verdict

@@ -0,0 +1,43 @@
+# ADR 0026 — A PVE behaviour claim documented against the published API spec is verified without the operator
+
+- **Status:** Accepted
+- **Date:** 2026-09-02
+- **Deciders:** operator + agent
+- **Context source:** wave 1 of the 2026-09-02 remediation; PRs #163 and #164; review of PR #170
+
+## Context
+
+The review prompt's "Reserved to the operator" list, as introduced by the commit [ADR 0025](0025-review-instructions-come-from-the-default-branch-and-review-governing-prs-cannot-self-approve.md) documents, reserves to the operator any PR that "claims live PVE behaviour that CI does not exercise". The rule exists because the offline suite proves payloads, not server semantics ([ADR 0021](0021-integration-tests-prove-server-semantics-payloads-are-proven-offline.md)), so a PR asserting what PVE does with a parameter is asserting something no check has verified.
+
+In wave 1 that rule deferred two PRs that were correct and green: #163 (the shape of `/access/permissions`) and #164 (`skiplock` and `force` on the remove endpoints). Both claims came straight from PVE's published API schema. Most behaviour-changing issues in the remaining waves make the same kind of claim, so the rule as written routes most of the remediation through the operator for facts that are already written down.
+
+The parsed PVE API specification lives in a public repository, `GoodOlClint/Proxmox_API`, as one OpenAPI file per PVE version under `pve/openapi/` plus a CHANGELOG of return-field history. The reviewer's tool allowlist already includes `gh api`, so it can read a file from that repository at a specific commit.
+
+The review prompt also says, of the enum fixtures, that "a value the schema accepts is not necessarily a value PVE acts on". That caveat is true of the published spec as well: it is the contract PVE publishes, not the server's behaviour, and the two can differ.
+
+## Decision
+
+A server-behaviour claim in a PR is one of three things: observed, documented, or inferred.
+
+- **Documented** means the PR cites a commit-anchored permalink into `GoodOlClint/Proxmox_API` (`.../blob/<sha>/pve/...`), to the version's OpenAPI spec file (a `#L` range locating the endpoint) or to the CHANGELOG entry for a return field. The reviewer fetches that file at that commit with `gh api` and reads it. If the spec supports the claim, the claim is verified and the review names the permalink it checked. Approval is not withheld for it.
+- **Inferred** means anything else: no citation, a branch-reference link, a link the reviewer cannot fetch, or a cited file that does not say what the PR says it does. Inferred claims stay reserved to the operator, exactly as ADR 0025 has them.
+
+The trade-off is taken with eyes open: the spec can lag or differ from the server. The check on that is the integration run at each wave end, which exercises main against a live cluster. When it fails on a spec-verified change, the named permalink shows the reviewer accepted the published contract, and the failure is a divergence between contract and server, which is itself worth knowing and reporting upstream. That is a better position than a guess that happened to be wrong.
+
+## Rejected alternatives
+
+**Keep every live-behaviour claim reserved to the operator.** Safe, and it is what ADR 0025 chose when there was no way for the reviewer to check the spec. It costs an operator approval per behaviour-changing PR, which in a 24-issue remediation is most of them, and it treats a claim read off the published schema the same as one made up.
+
+**Narrow the skip to non-destructive operations only.** Considered on review of #170. It does not follow from the evidence: the spec is no more or less reliable for a `DELETE` than for a `GET`, and the destructive cmdlets already carry `ConfirmImpact.High` and the operator's integration run. A narrowing on operation type would add a second taxonomy for the reviewer to apply without changing what is actually known.
+
+**Let the reviewer verify against the local `~/Source/pve_api` checkout.** Unreachable from the runner; only a GitHub-hosted copy is.
+
+**Accept branch-reference links.** Mutable. The traceability the decision depends on holds only if the citation is pinned to a commit.
+
+## Consequences
+
+- The review prompt's "API correctness" item and the "Reserved to the operator" list carry the three-way taxonomy, the permalink form, and the `gh api` invocation.
+- The agent contract for remediation waves requires the permalink in every PR body that makes a behaviour claim.
+- `GoodOlClint/Proxmox_API` becomes part of the review oracle alongside the enum fixtures. A PR to this repository cannot alter it, which is the property the fixtures lack and why they are code-owned.
+- Wave-end integration runs are load-bearing for this decision and stay in the plan; a failure on a spec-verified change is recorded against the permalink, not dismissed as a flake.
+- ADR 0025 is not superseded. The review prompt's "claims live PVE behaviour" bullet, which that ADR's commit introduced, now reads as "claims live PVE behaviour that CI does not exercise and the PR does not document against the spec".