fix: compare JSON nesting, list null array, hold-remove false success + 1.0.10 release prep

CHANGELOG.md

@@ -11,6 +11,38 @@ keeps reading that version for at least 24 months after a successor lands.
 
 ## [Unreleased]
 
+## [1.0.10] — 2026-07-15
+
+### Fix: `recovery drill` failed every WAL-streaming backup (#26)
+
+The verify sandbox that `recovery drill` and `verify --full` use ran
+`pg_verifybackup` without `-n`/`--no-parse-wal`. pg_hardstorage stores
+WAL in the repository rather than inside the base backup, so the
+restored data directory legitimately has an empty `pg_wal/` — the
+WAL-parse step therefore failed every structurally-valid WAL-streaming
+backup with `could not find any WAL file`, and the drill reported
+`verdict: fail` for backups that restore, recover, and serve data
+correctly. The sandbox now verifies the manifest and file checksums
+with `-n`, matching the restore path's `--verify` gate (the same
+defect was fixed there in 1.0.8).
+
+### Fix: `hold remove` reported success for holds that don't exist
+
+Releasing a hold with a typo'd backup ID printed `✓ Hold released`
+and exited 0 while the real hold silently kept blocking retention — a
+false success on the legal-hold path. Removal of a nonexistent hold
+now fails with `notfound.hold` (exit 6) and points at `hold list`;
+releasing an existing hold is unchanged.
+
+### Fix: JSON output shape papercuts
+
+- `backup compare -o json` double-nested its payload under
+  `.result.result.*`; the comparison fields now sit at `.result.*`
+  like every other command.
+- `list` on a deployment with no backups emitted `"backups": null`;
+  it now emits `[]`, so `jq '.result.backups[]'` and every other
+  iterator handle the empty case.
+
 ## [1.0.9] — 2026-07-13
 
 Twenty operator-annoyance fixes, found by systematically exercising the

docs/compliance/pci-dss.md

@@ -143,7 +143,7 @@ audit period plus the cosign signatures on the running
 binary:
 
 ```sh
-VERSION=1.0.9   # the release / image tag you're attesting
+VERSION=1.0.10   # the release / image tag you're attesting
 
 # 1. Audit chain bundle
 pg_hardstorage audit export-bundle \

docs/compliance/slsa-l3-provenance.md

@@ -74,7 +74,7 @@ Verify a downloaded artifact against its provenance with the
 `slsa-verifier`:
 
 ```sh
-VERSION=1.0.9   # the release you're verifying
+VERSION=1.0.10   # the release you're verifying
 slsa-verifier verify-artifact \
     --provenance-path pg_hardstorage.intoto.jsonl \
     --source-uri github.com/cybertec-postgresql/pg_hardstorage \
@@ -109,7 +109,7 @@ When images ship, they are intended to carry a cosign
 signature plus an image-level SLSA provenance attestation:
 
 ```sh
-VERSION=1.0.9   # the image tag you're verifying
+VERSION=1.0.10   # the image tag you're verifying
 
 # Verify the image signature
 cosign verify "ghcr.io/cybertec-postgresql/pg_hardstorage:v${VERSION}" \
@@ -139,7 +139,7 @@ JSON format, generated by `syft` (configured via the
 and attached to the GitHub Release:
 
 ```sh
-VERSION=1.0.9   # the release you're inspecting
+VERSION=1.0.10   # the release you're inspecting
 # The SBOM is a plain file on the Release; inspect it directly
 jq -r '.packages[].name' \
     "pg_hardstorage_${VERSION}_linux_amd64.tar.gz.spdx.sbom.json"

docs/reference/runbooks/control-plane-setup.md

@@ -249,7 +249,7 @@ curl --cacert /etc/pg_hardstorage/server/cert.pem \
 #       {
 #         "id": "db1.example.com",
 #         "host": "db1.example.com",
-#         "version": "v1.0.9",
+#         "version": "v1.0.10",
 #         "deployments": ["db1", "db2"],
 #         "registered_at": "...",
 #         "last_heartbeat": "..."

docs/tutorials/getting-started.md

@@ -35,7 +35,7 @@ tarballs (Windows is CLI-only). Grab the matching one from
 verify the cosign signature, and drop the binary on your `$PATH`:
 
 ```sh
-VERSION=1.0.9   # latest release: https://github.com/cybertec-postgresql/pg_hardstorage/releases/latest
+VERSION=1.0.10   # latest release: https://github.com/cybertec-postgresql/pg_hardstorage/releases/latest
 curl -LO "https://github.com/cybertec-postgresql/pg_hardstorage/releases/download/v${VERSION}/pg_hardstorage_${VERSION}_linux_amd64.tar.gz"
 tar xzf "pg_hardstorage_${VERSION}_linux_amd64.tar.gz"
 sudo install -m 0755 pg_hardstorage /usr/local/bin/
@@ -45,7 +45,7 @@ pg_hardstorage version
 ### `.deb` (Debian / Ubuntu)
 
 ```sh
-VERSION=1.0.9   # the release you downloaded
+VERSION=1.0.10   # the release you downloaded
 sudo dpkg -i "pg-hardstorage_${VERSION}_amd64.deb"
 ```
 
@@ -57,7 +57,7 @@ creates `/etc/pg_hardstorage/`, `/var/lib/pg_hardstorage/`,
 ### `.rpm` (Fedora / RHEL / Rocky / Alma)
 
 ```sh
-VERSION=1.0.9   # the release you downloaded
+VERSION=1.0.10   # the release you downloaded
 sudo rpm -i "pg-hardstorage-${VERSION}-1.x86_64.rpm"
 ```
 
@@ -259,7 +259,7 @@ you are doing — exit 9 means the verifier said no.
 
 ```sh
 $ pg_hardstorage version
-pg_hardstorage v1.0.9 (abc1234, built 2026-04-29T12:00:00Z)
+pg_hardstorage v1.0.10 (abc1234, built 2026-04-29T12:00:00Z)
 ```
 
 `doctor` is the single-command "is anything wrong" check. It prints a

internal/cli/backup_compare.go

@@ -110,8 +110,8 @@ func runBackupCompare(cmd *cobra.Command, deployment, idA, idB, repoURL string,
 	}
 
 	return d.Result(output.NewResult(cmd.CommandPath()).WithBody(backupCompareBody{
-		Deployment: deployment,
-		Result:     res,
+		Deployment:       deployment,
+		ComparisonResult: res,
 	}))
 }
 
@@ -140,18 +140,20 @@ func mapCompareReadError(side, deployment, backupID string, err error) error {
 		fmt.Sprintf("backup compare: %s side: %v", side, err)).Wrap(err)
 }
 
-// backupCompareBody is the v1-stable result shape. Embeds the
-// ComparisonResult plus the deployment name (since the result
-// itself only carries per-side BackupID).
+// backupCompareBody is the v1-stable result shape: the
+// ComparisonResult's fields inlined (via embedding) alongside the
+// deployment name. It previously wrapped the result under a nested
+// "result" key, so JSON consumers saw the body at `.result.result.*`
+// — double-nested relative to every sibling command.
 type backupCompareBody struct {
-	Deployment string                   `json:"deployment"`
-	Result     *backup.ComparisonResult `json:"result"`
+	Deployment string `json:"deployment"`
+	*backup.ComparisonResult
 }
 
 // WriteText renders the comparison as compact tables: per-side
 // summary, file-class counts, chunk-class counts, top-N deltas.
 func (b backupCompareBody) WriteText(w io.Writer) error {
-	r := b.Result
+	r := b.ComparisonResult
 	bw := &strings.Builder{}
 	fmt.Fprintf(bw, "compare %s/%s ↔ %s\n", b.Deployment, r.A.BackupID, r.B.BackupID)
 

internal/cli/hold.go

@@ -444,12 +444,26 @@ func runHoldRemove(cmd *cobra.Command, deployment, backupID, repoURL string, yes
 	defer sp.Close()
 
 	store := backup.NewManifestStore(sp)
-	// Capture the hold's holder/reason BEFORE removing it so the audit
-	// record can say whose hold was released and why.
-	var prevHolder, prevReason string
-	if h, gerr := store.GetHold(cmd.Context(), deployment, backupID); gerr == nil && h != nil {
-		prevHolder, prevReason = h.Holder, h.Reason
+	// Read the hold BEFORE removing it — both to stamp the audit record
+	// with whose hold was released and why, and to refuse a release of a
+	// hold that does not exist. The storage-layer RemoveHold is
+	// deliberately idempotent, so without this check a typo'd backup ID
+	// printed "✓ Hold released" with exit 0 while the real hold silently
+	// kept blocking retention — a false success on the legal-hold path.
+	h, gerr := store.GetHold(cmd.Context(), deployment, backupID)
+	switch {
+	case errors.Is(gerr, storage.ErrNotFound):
+		return output.NewError("notfound.hold",
+			fmt.Sprintf("hold remove: no hold exists on %s/%s", deployment, backupID)).
+			WithSuggestion(&output.Suggestion{
+				Human:   "check the exact backup ID with `pg_hardstorage hold list " + deployment + "`",
+				Command: "pg_hardstorage hold list " + deployment + " --repo " + repoURL,
+			})
+	case gerr != nil:
+		return output.NewError("hold.read_failed",
+			fmt.Sprintf("hold remove: read hold %s/%s: %v", deployment, backupID, gerr)).Wrap(gerr)
 	}
+	prevHolder, prevReason := h.Holder, h.Reason
 	if err := store.RemoveHold(cmd.Context(), deployment, backupID); err != nil {
 		return output.NewError("hold.remove_failed",
 			fmt.Sprintf("hold remove: %v", err)).Wrap(err)

internal/cli/hold_extern_test.go

@@ -67,14 +67,21 @@ func TestHold_Remove_RequiresYes(t *testing.T) {
 	}
 }
 
-func TestHold_Remove_Idempotent(t *testing.T) {
+func TestHold_Remove_AbsentHoldFails(t *testing.T) {
 	w := newReadWorld(t)
 	id := commitVerifiableBackup(t, w, "db1", 0, []byte("body"))
-	// No hold placed; removing should still succeed (idempotent).
-	_, _, exit := runCLI(t, "hold", "remove", "db1", id,
+	// No hold placed; removal must FAIL with notfound.hold. The old
+	// contract (exit 0, "✓ Hold released") was a false success on the
+	// legal-hold path — a typo'd backup ID confirmed a release while
+	// the real hold silently kept blocking retention. Idempotency
+	// lives at the storage layer, not in the operator-facing verb.
+	stdout, stderr, exit := runCLI(t, "hold", "remove", "db1", id,
 		"--repo", w.repoURL, "--yes", "-o", "json")
-	if exit != int(output.ExitOK) {
-		t.Errorf("remove of absent hold should be idempotent (exit 0); got %d", exit)
+	if exit != int(output.ExitNotFound) {
+		t.Errorf("remove of absent hold: exit = %d, want %d (notfound)", exit, output.ExitNotFound)
+	}
+	if !strings.Contains(stdout+stderr, "notfound.hold") {
+		t.Errorf("expected notfound.hold in output:\n%s%s", stdout, stderr)
 	}
 }
 

internal/cli/hold_remove_notfound_test.go

@@ -0,0 +1,30 @@
+package cli_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/cybertec-postgresql/pg_hardstorage/internal/output"
+)
+
+// Regression: `hold remove --yes` on a hold that never existed printed
+// "✓ Hold released" with exit 0 (the storage layer's RemoveHold is
+// idempotent) — a false success on the legal-hold path: typo one
+// character of the backup ID and the real hold silently keeps blocking
+// retention. It must fail with notfound.hold.
+func TestHoldRemove_NonexistentHoldFails(t *testing.T) {
+	repoURL := "file://" + t.TempDir() + "/repo"
+	if _, _, exit := runCLI(t, "repo", "init", repoURL); exit != int(output.ExitOK) {
+		t.Fatalf("repo init failed")
+	}
+	out, errOut, exit := runCLI(t,
+		"hold", "remove", "db1", "db1.full.20990101T000000Z.dead",
+		"--repo", repoURL, "--yes", "-o", "json")
+	combined := out + errOut
+	if exit == 0 {
+		t.Fatalf("hold remove on nonexistent hold exited 0 (false success):\n%s", combined)
+	}
+	if !strings.Contains(combined, "notfound.hold") {
+		t.Errorf("expected notfound.hold, got exit=%d:\n%s", exit, combined)
+	}
+}

internal/cli/json_shape_test.go

@@ -0,0 +1,56 @@
+package cli
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/cybertec-postgresql/pg_hardstorage/internal/backup"
+)
+
+// Regression: backupCompareBody used to wrap ComparisonResult under a
+// nested "result" key, so consumers saw the payload at
+// `.result.result.*` — double-nested relative to every sibling command.
+// The fields must inline next to "deployment".
+func TestBackupCompareBody_JSONNotDoubleNested(t *testing.T) {
+	b := backupCompareBody{
+		Deployment:       "db1",
+		ComparisonResult: &backup.ComparisonResult{},
+	}
+	raw, err := json.Marshal(b)
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	var m map[string]any
+	if err := json.Unmarshal(raw, &m); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if _, nested := m["result"]; nested {
+		t.Errorf("compare body still nests under \"result\": %s", raw)
+	}
+	if _, ok := m["deployment"]; !ok {
+		t.Errorf("compare body lost \"deployment\": %s", raw)
+	}
+}
+
+// Regression: an empty deployment produced `"backups": null` (nil slice)
+// — breaking every consumer that iterates `.result.backups[]`. The empty
+// case must marshal as [].
+func TestListBody_EmptyBackupsIsArray(t *testing.T) {
+	body := listBody{Deployment: "db1", Backups: []backupSummary{}}
+	raw, err := json.Marshal(body)
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	var m map[string]any
+	if err := json.Unmarshal(raw, &m); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	arr, ok := m["backups"].([]any)
+	if !ok || arr == nil {
+		t.Errorf("backups = %v (%T), want empty JSON array", m["backups"], m["backups"])
+	}
+}
+
+// (Regression for the third inconvenience fix lives in
+// hold_remove_notfound_test.go — kept separate: it needs the CLI
+// runner + a live repo fixture.)

internal/cli/list.go

@@ -132,6 +132,12 @@ func runList(cmd *cobra.Command, deployment, repoURL string, includeDeleted, onl
 	sort.Slice(summaries, func(i, j int) bool {
 		return summaries[i].StoppedAt.After(summaries[j].StoppedAt)
 	})
+	// A nil slice marshals as JSON null, and `"backups": null` breaks
+	// every consumer iterating `.result.backups[]` — precisely on the
+	// empty-deployment case scripts probe first. Always emit [].
+	if summaries == nil {
+		summaries = []backupSummary{}
+	}
 
 	body := listBody{
 		Deployment:          deployment,