reconcile: tier-aware restore-task deadline (S4.1 unattended offsite restore-test)

A WAN (pbs-tier) restore of a large guest exceeds the restore-task wait's 10m
default → the wait expired mid-restore, teardown fired against a still-restoring
(not-yet-pool-associated) scratch guest → leak + a phantom VM.Allocate 403.

- RestoreTestSpec.RestoreTaskTimeout (0→10m default); the restore WaitTask passes
  it. Local tier unchanged (10m).
- config RestoreTestPBSRestoreTimeoutSeconds + accessor (default 120m).
- main restoreTaskTimeout(cfg,tier): configured PBS timeout only when tier==pbs,
  else 0. Both scheduler + selftest spec builds.
- Tests + WaitOptions red-proof + accessor contract.

The "grant scratch-band VM.Allocate" follow-up is diagnosed not blind-applied:
the scratch is restored INTO /pool/felhom (ACL already grants VM.Allocate), so
the earlier 403 was a consequence of the timeout. No ACL/host-install change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6

CHANGELOG.md

@@ -1,3 +1,25 @@
+## v0.68.0 — S4.1: tier-aware restore-task deadline (unattended offsite restore-test) (2026-07-04)
+
+The offsite restore-test couldn't complete on the scheduler path because a WAN restore of a large
+guest exceeds the restore-task wait's 10-minute default — the wait expired mid-restore, teardown
+then fired against a still-restoring (not-yet-pool-associated) scratch guest, and it leaked. Make
+the restore-task wait **tier-aware**.
+
+- **`internal/reconcile`**: `RestoreTestSpec.RestoreTaskTimeout` (0 → the 10m `WaitOptions` default);
+  the restore-task `WaitTask` now passes it. Local-tier restores are UNCHANGED (10m — a local
+  restore hanging that long is a genuine fault).
+- **`internal/config`**: `BackupConfig.RestoreTestPBSRestoreTimeoutSeconds` + accessor
+  `RestoreTestPBSRestoreTimeout()` (positive as-is, else **120m** — for an unattended nightly test a
+  false timeout is worse than a slow pass; very large guests may need more).
+- **`cmd/felhom-agent`**: `restoreTaskTimeout(cfg, tier)` sets the field to the configured PBS
+  timeout only when `SourceTier=="pbs"` (both the scheduler + selftest spec builds), else 0.
+- Tests: tier-aware `WaitOptions.Timeout` (pbs→120m, local→0; red-proofed against `WaitOptions{}`) +
+  the accessor contract. The "grant scratch-band `VM.Allocate`" follow-up was diagnosed, not
+  blind-applied — the scratch guest is restored INTO `/pool/felhom` (whose ACL already grants
+  `VM.Allocate`), so the earlier teardown 403 was a *consequence* of the timeout (a not-yet-pooled,
+  still-restoring guest), not a missing grant. **No ACL/host-install change.** (Live confirmation of
+  the phantom in REPORT.)
+
 ## v0.67.0 — S4: namespace-aware PBS client (per-customer offsite tenancy) (2026-07-04)
 
 Phase-1 live probe on felhom-hetzner proved the offsite tenancy path (backup/restore/list/isolation

cmd/felhom-agent/main.go

@@ -779,6 +779,16 @@ func storageTier(ctx context.Context, px *proxmox.Client, storageID string) stri
 	return "local"
 }
 
+// restoreTaskTimeout returns the tier-aware restore-task wait: the generous configured PBS timeout
+// for a WAN (pbs-tier) restore, else 0 (→ WaitOptions' 10m default) for a local restore. S4.1: a
+// too-short wait kills a WAN restore mid-flight → mid-restore teardown → leaked scratch.
+func restoreTaskTimeout(cfg config.Config, tier string) time.Duration {
+	if tier == "pbs" {
+		return cfg.Backup.RestoreTestPBSRestoreTimeout()
+	}
+	return 0
+}
+
 // readTrimmed reads a file and trims surrounding whitespace/newline (for the .pw secret).
 func readTrimmed(path string) (string, error) {
 	b, err := os.ReadFile(path)
@@ -811,12 +821,16 @@ func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *re
 		Runner: engine,
 		Pick:   runner.PickRestoreCandidate,
 		Store:  store,
-		Spec: reconcile.RestoreTestSpec{
-			RestoreStorage: cfg.Backup.RestoreStorage,
-			ScratchMin:     min,
-			ScratchMax:     max,
-			SourceTier:     storageTier(context.Background(), px, target),
-		},
+		Spec: func() reconcile.RestoreTestSpec {
+			tier := storageTier(context.Background(), px, target)
+			return reconcile.RestoreTestSpec{
+				RestoreStorage:     cfg.Backup.RestoreStorage,
+				ScratchMin:         min,
+				ScratchMax:         max,
+				SourceTier:         tier,
+				RestoreTaskTimeout: restoreTaskTimeout(cfg, tier),
+			}
+		}(),
 		Cadence: cadence,
 		Logger:  logger,
 	})
@@ -1249,9 +1263,11 @@ func runSelftestRestoreTest(ctx context.Context, cfg config.Config, logger *slog
 	}
 	min, max := cfg.Backup.ScratchBand()
 	fmt.Printf("  restoring %s into scratch band [%d,%d] on %s …\n", archive, min, max, cfg.Backup.RestoreStorage)
+	rtTier := storageTier(ctx, px, target)
 	res := engine.RunRestoreTest(ctx, reconcile.RestoreTestSpec{
 		Archive: archive, RestoreStorage: cfg.Backup.RestoreStorage,
-		ScratchMin: min, ScratchMax: max, SourceTier: storageTier(ctx, px, target),
+		ScratchMin: min, ScratchMax: max, SourceTier: rtTier,
+		RestoreTaskTimeout: restoreTaskTimeout(cfg, rtTier),
 	})
 	printJSON("restore-test record", backup.ToHubRestoreTest(res, time.Now().UTC()))
 	if res.Skipped {

internal/config/config.go

@@ -198,6 +198,12 @@ type BackupConfig struct {
 	// always excluded. Defaults to 990000–990009.
 	ScratchVMIDMin int `json:"scratch_vmid_min"`
 	ScratchVMIDMax int `json:"scratch_vmid_max"`
+	// RestoreTestPBSRestoreTimeoutSeconds bounds the wait on a PBS-tier (offsite/WAN) restore-test
+	// restore task; 0 → default 120m. A large guest restored over a slow home uplink runs long, and
+	// for an UNATTENDED nightly test a false timeout (→ mid-restore teardown → leaked scratch) is
+	// worse than a slow pass. Very large guests may need a higher value. LOCAL-tier restores keep
+	// the 10m WaitOptions default (a local restore hanging 10m is a genuine fault).
+	RestoreTestPBSRestoreTimeoutSeconds int `json:"restore_test_pbs_restore_timeout_seconds"`
 
 	// PBS (slice 6 Phase B). The verify maintenance loop runs on its own cadence (cheaper +
 	// more frequent than the full restore-test); 0 → default (6h), negative → disabled.
@@ -245,6 +251,14 @@ func (b BackupConfig) BackupCadence() time.Duration {
 	return 24 * time.Hour
 }
 
+// RestoreTestPBSRestoreTimeout returns the PBS-tier restore-task wait: positive as-is, else 120m.
+func (b BackupConfig) RestoreTestPBSRestoreTimeout() time.Duration {
+	if b.RestoreTestPBSRestoreTimeoutSeconds > 0 {
+		return time.Duration(b.RestoreTestPBSRestoreTimeoutSeconds) * time.Second
+	}
+	return 120 * time.Minute
+}
+
 // defaultBackupTarget is the offsite PBS storage whole-guest backups land on by default. It is
 // SEPARATE HARDWARE from the guest's own disk (a PBS datastore on the DooPlex box), so a host
 // disk/hardware failure doesn't take the backups with it — that's what makes it real DR. Proven

internal/config/config_test.go

@@ -5,8 +5,29 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 )
 
+// TestRestoreTestPBSRestoreTimeout mirrors the BackupCadence accessor contract: positive as-is,
+// 0 → default (120m), negative → default.
+func TestRestoreTestPBSRestoreTimeout(t *testing.T) {
+	cases := []struct {
+		secs int
+		want time.Duration
+	}{
+		{0, 120 * time.Minute},
+		{-5, 120 * time.Minute},
+		{1800, 30 * time.Minute},
+		{7200, 120 * time.Minute},
+	}
+	for _, c := range cases {
+		got := BackupConfig{RestoreTestPBSRestoreTimeoutSeconds: c.secs}.RestoreTestPBSRestoreTimeout()
+		if got != c.want {
+			t.Errorf("RestoreTestPBSRestoreTimeout(secs=%d) = %v, want %v", c.secs, got, c.want)
+		}
+	}
+}
+
 func TestRedactedMasksSecret(t *testing.T) {
 	c := Default()
 	c.Proxmox.Token = "felhom-agent@pve!agent=b6547d9d-08ec-4f22-beb8-a551dc2cd69d"

internal/reconcile/engine_test.go

@@ -47,6 +47,7 @@ type fakeAPI struct {
 	restores []proxmox.RestoreLXCOptions
 	destroys []int
 	waits    []string
+	waitOpts []proxmox.WaitOptions // parallel to waits: the options each WaitTask was called with
 	listErr  error
 }
 
@@ -151,9 +152,10 @@ func (f *fakeAPI) ResizeLXC(_ context.Context, vmid int, disk, size string) (str
 	return f.resizeUPID, f.resizeErr
 }
 
-func (f *fakeAPI) WaitTask(_ context.Context, upid string, _ proxmox.WaitOptions) (proxmox.TaskStatus, error) {
+func (f *fakeAPI) WaitTask(_ context.Context, upid string, opts proxmox.WaitOptions) (proxmox.TaskStatus, error) {
 	f.mu.Lock()
 	f.waits = append(f.waits, upid)
+	f.waitOpts = append(f.waitOpts, opts)
 	f.mu.Unlock()
 	if f.waitFunc != nil {
 		return f.waitFunc(upid)

internal/reconcile/restoretest.go

@@ -35,6 +35,11 @@ type RestoreTestSpec struct {
 	ScratchMin     int           // inclusive scratch VMID band (must be > 0)
 	ScratchMax     int           // inclusive
 	BootTimeout    time.Duration // 0 → DefaultBootTimeout
+	// RestoreTaskTimeout bounds the wait on the restore (vzrestore) task. 0 → WaitOptions' 10m
+	// default (fine for a LOCAL restore). A WAN/pbs restore of a large guest runs long, so the
+	// caller sets this generously for the pbs tier — else the wait expires mid-restore, teardown
+	// fires against a still-restoring (not-yet-pool-associated) guest, and the scratch leaks.
+	RestoreTaskTimeout time.Duration
 }
 
 // RestoreTestResult is the reconcile-local outcome (the backup package maps it to the
@@ -243,7 +248,7 @@ func (e *Engine) runScratchTest(ctx context.Context, vmid int, spec RestoreTestS
 	launched = true
 	e.append(withUPID(base, upid, OpTaskRunning))
 	if upid != "" {
-		if _, err := e.api.WaitTask(ctx, upid, proxmox.WaitOptions{}); err != nil {
+		if _, err := e.api.WaitTask(ctx, upid, proxmox.WaitOptions{Timeout: spec.RestoreTaskTimeout}); err != nil {
 			res.Err = fmt.Errorf("reconcile: restore-test restore task: %w", err)
 			return
 		}

internal/reconcile/restoretest_test.go

@@ -56,6 +56,49 @@ func TestRunRestoreTest_PassAndTeardown(t *testing.T) {
 	}
 }
 
+// TestRunRestoreTest_TierAwareRestoreTimeout (S4.1) pins that the restore-task wait carries the
+// tier-derived timeout: the configured (generous) value for a pbs/WAN restore, and 0 (→ the 10m
+// WaitOptions default, UNCHANGED) for a local restore. Red-proof: revert the L246 wait to
+// WaitOptions{} → the pbs assertion (120m) fails.
+func TestRunRestoreTest_TierAwareRestoreTimeout(t *testing.T) {
+	const restoreUPID = "UPID:node:1:2:3:4:vzrestore:990000:tok:" // async restore → the wait fires
+
+	restoreWaitTimeout := func(api *fakeAPI) (time.Duration, bool) {
+		for i, u := range api.waits {
+			if u == restoreUPID {
+				return api.waitOpts[i].Timeout, true
+			}
+		}
+		return 0, false
+	}
+
+	// pbs tier → the configured generous timeout is passed to WaitTask.
+	pbsAPI := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}, restoreUPID: restoreUPID}
+	e, _, q := newEngine(t, pbsAPI, EmptyProvider{})
+	defer q.Close()
+	e.RunRestoreTest(context.Background(), RestoreTestSpec{
+		Archive: "felhom-offsite:backup/ct/9201/x", RestoreStorage: "local-lvm",
+		ScratchMin: 990000, ScratchMax: 990009, SourceTier: "pbs",
+		RestoreTaskTimeout: 120 * time.Minute,
+	})
+	if to, ok := restoreWaitTimeout(pbsAPI); !ok || to != 120*time.Minute {
+		t.Errorf("pbs restore wait Timeout = %v (found=%v), want 120m", to, ok)
+	}
+
+	// local tier → 0 (→ WaitOptions' 10m default preserved, UNCHANGED).
+	localAPI := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}, restoreUPID: restoreUPID}
+	e2, _, q2 := newEngine(t, localAPI, EmptyProvider{})
+	defer q2.Close()
+	e2.RunRestoreTest(context.Background(), RestoreTestSpec{
+		Archive: "local:backup/x.tar.zst", RestoreStorage: "local-lvm",
+		ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
+		RestoreTaskTimeout: 0,
+	})
+	if to, ok := restoreWaitTimeout(localAPI); !ok || to != 0 {
+		t.Errorf("local restore wait Timeout = %v (found=%v), want 0 (→10m default)", to, ok)
+	}
+}
+
 // startWarnAPI builds a fakeAPI whose guest-start task exits "WARNINGS: 1" and whose start
 // task log contains the given warning lines. The guest reaches running (status default).
 func startWarnAPI(startUPID string, logLines []string) *fakeAPI {