Merge pull request 'Release v1.1.1 — forget reliability fixes' (#42)

CHANGELOG.md

@@ -6,6 +6,23 @@ and the project follows [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+## [1.1.1] - 2026-08-22
+
+### Fixed
+
+- Prevented agents from panicking when a WebSocket connection ends. The
+  WebSocket library transfers ownership of a successful upgrade stream to
+  the connection and leaves the HTTP response body nil; attempting to close
+  that body caused agents to restart and left forget jobs permanently stuck
+  in `running`. ([#37])
+- Manual forget jobs now receive the same per-source-group retention policies
+  as scheduled forget jobs. The API also supports a validated `--dry-run`
+  option through the complete server-to-agent-to-restic path, and rejects
+  hosts without configured retention before creating a job. ([#36])
+- Corrected the admin-credentials help text to reflect that forget uses normal
+  append-only credentials and blank admin credentials do not provide a
+  fallback for prune. ([#34])
+
 ## [1.1.0] - 2026-06-15
 
 ### Added
@@ -123,5 +140,11 @@ with a web UI, JSON API, and self-updating agent fleet.
   go vet, golangci-lint).
 - Threat model published (`docs/threat-model.md`).
 
-[Unreleased]: https://gitea.dcglab.co.uk/steve/restic-manager/compare/v1.0.0...HEAD
+[Unreleased]: https://gitea.dcglab.co.uk/steve/restic-manager/compare/v1.1.1...HEAD
+[1.1.1]: https://gitea.dcglab.co.uk/steve/restic-manager/compare/v1.1.0...v1.1.1
+[1.1.0]: https://gitea.dcglab.co.uk/steve/restic-manager/releases/tag/v1.1.0
 [1.0.0]: https://gitea.dcglab.co.uk/steve/restic-manager/releases/tag/v1.0.0
+
+[#37]: https://gitea.dcglab.co.uk/steve/restic-manager/issues/37
+[#36]: https://gitea.dcglab.co.uk/steve/restic-manager/issues/36
+[#34]: https://gitea.dcglab.co.uk/steve/restic-manager/issues/34

CONTRIBUTING.md

@@ -2,7 +2,8 @@
 
 Thanks for your interest in restic-manager. This document covers how
 to set up a development environment, the conventions the project
-follows, and how patches make it from your machine into `main`.
+follows, and how to contribute through issues as well as patches that
+make it from your machine into `main`.
 
 ## Project status and scope
 
@@ -108,6 +109,32 @@ admin user.
 
 ## Workflow
 
+### Opening an issue
+
+Issues are contributions too. Use them to report a bug, suggest a
+feature, improve the documentation, or start a design discussion even
+if you do not plan to submit a patch.
+
+Before opening one, search the existing issues and check `tasks.md` to
+see whether the topic is already tracked. Then choose the closest issue
+template:
+
+- [Bug report](./.gitea/issue_template/bug_report.md) for behaviour that
+  does not match the documentation or expected operation.
+- [Feature request](./.gitea/issue_template/feature_request.md) for a new
+  capability or a change to existing behaviour.
+
+Give the issue a specific title, keep it to one problem or proposal,
+and complete the relevant template fields. If no template is an exact
+fit, open a regular issue and explain the context, desired outcome, and
+any alternatives you have considered. Maintainers may ask follow-up
+questions or close requests that duplicate existing work or fall
+outside the project's scope.
+
+Security-sensitive reports are the exception: follow the
+[SECURITY.md](./SECURITY.md) disclosure process and do not open a public
+issue.
+
 ### Before opening a PR
 
 1. **Open an issue first** for non-trivial changes. The design is
@@ -136,25 +163,24 @@ The PR template asks for:
 
 ### Reporting bugs
 
-Open an issue with:
+Use the bug report issue template and include:
 
 - restic-manager version (`server --version`) and agent version.
 - restic version on the affected host.
 - Steps to reproduce.
 - Server and agent logs (sanitise any tokens before pasting).
 
-Security-sensitive bugs go through the [SECURITY.md](./SECURITY.md)
-disclosure path instead — please don't open a public issue for
-them.
+For security-sensitive bugs, use the private disclosure process noted
+above.
 
 ### Suggesting features
 
-Open an issue describing the use case (not just the proposed
-solution). The roadmap in `tasks.md` shows where the project is
-heading; if the suggestion fits a future phase we'll wire it in
-there. If it falls outside the project's scope (multi-tenancy, SaaS,
-non-restic backends — see `spec.md` §2 non-goals) we'll say so
-early to save your time.
+Use the feature request issue template and describe the use case (not
+just the proposed solution). The roadmap in `tasks.md` shows where the
+project is heading; if the suggestion fits a future phase we'll wire it
+in there. If it falls outside the project's scope (multi-tenancy, SaaS,
+non-restic backends — see `spec.md` §2 non-goals) we'll say so early to
+save your time.
 
 ## Code of conduct
 

cmd/agent/main.go

@@ -601,6 +601,11 @@ func (d *dispatcher) runJob(ctx context.Context, p api.CommandRunPayload, tx wsc
 			failJob(p, tx, "forget: command.run carried no forget_groups (server didn't populate them)")
 			return fmt.Errorf("forget: command.run carried no forget_groups (server didn't populate them)")
 		}
+		if len(p.Args) > 1 || (len(p.Args) == 1 && p.Args[0] != "--dry-run") {
+			failJob(p, tx, "forget: command.run carried unsupported arguments")
+			return fmt.Errorf("forget: command.run carried unsupported arguments")
+		}
+		dryRun := len(p.Args) == 1
 		groups := make([]restic.ForgetGroup, 0, len(p.ForgetGroups))
 		for _, g := range p.ForgetGroups {
 			groups = append(groups, restic.ForgetGroup{
@@ -615,9 +620,9 @@ func (d *dispatcher) runJob(ctx context.Context, p api.CommandRunPayload, tx wsc
 				},
 			})
 		}
-		slog.Info("agent: accepting forget job", "job_id", p.JobID, "groups", len(groups))
+		slog.Info("agent: accepting forget job", "job_id", p.JobID, "groups", len(groups), "dry_run", dryRun)
 		spawn("forget", func(jobCtx context.Context) error {
-			return r.RunForget(jobCtx, p.JobID, groups)
+			return r.RunForget(jobCtx, p.JobID, groups, dryRun)
 		})
 	case api.JobPrune:
 		// Prune may require admin creds (delete authority on rest-server).

internal/agent/runner/runner.go

@@ -274,13 +274,13 @@ func (r *Runner) RunInit(ctx context.Context, jobID string) error {
 // snapshot projection (forget rewrites the snapshot index — the
 // host's snapshot list shrinks). Snapshot refresh runs once after
 // every group completes, not per-group.
-func (r *Runner) RunForget(ctx context.Context, jobID string, groups []restic.ForgetGroup) error {
+func (r *Runner) RunForget(ctx context.Context, jobID string, groups []restic.ForgetGroup, dryRun bool) error {
 	startedAt := time.Now().UTC()
 	r.sendStarted(jobID, api.JobForget, startedAt)
 
 	env := r.resticEnv()
 	var seq atomic.Int64
-	err := env.RunForget(ctx, groups, r.streamHandler(jobID, &seq))
+	err := env.RunForget(ctx, groups, dryRun, r.streamHandler(jobID, &seq))
 	finishedAt := time.Now().UTC()
 	r.sendFinished(ctx, jobID, finishedAt, err, nil)
 

internal/agent/runner/runner_test.go

@@ -398,7 +398,7 @@ esac
 		Tag:    "documents",
 		Policy: restic.ForgetPolicy{KeepLast: &keepLast},
 	}}
-	if err := r.RunForget(context.Background(), "job-forget", groups); err != nil {
+	if err := r.RunForget(context.Background(), "job-forget", groups, false); err != nil {
 		t.Fatalf("RunForget: %v", err)
 	}
 	_ = firstEnvOfType(t, tx.envs, api.MsgJobStarted)

internal/restic/runner.go

@@ -322,7 +322,7 @@ type ForgetGroup struct {
 // any keep-* would delete every snapshot in the tagged set).
 // Returns the first error encountered, or nil when every group runs
 // to a clean exit.
-func (e Env) RunForget(ctx context.Context, groups []ForgetGroup, handle LineHandler) error {
+func (e Env) RunForget(ctx context.Context, groups []ForgetGroup, dryRun bool, handle LineHandler) error {
 	if len(groups) == 0 {
 		return fmt.Errorf("restic forget: refusing to run with no groups (would be a no-op)")
 	}
@@ -332,6 +332,9 @@ func (e Env) RunForget(ctx context.Context, groups []ForgetGroup, handle LineHan
 		}
 		args := []string{"forget", "--json", "--tag", g.Tag}
 		args = append(args, g.Policy.args()...)
+		if dryRun {
+			args = append(args, "--dry-run")
+		}
 		cmd := e.resticCmd(ctx, args...)
 		if err := runWithPump(cmd, handle); err != nil {
 			return err

internal/restic/runner_test.go

@@ -60,6 +60,26 @@ func TestRunPruneInvokesPrune(t *testing.T) {
 	t.Fatalf("expected 'prune' in captured output; got: %v", *lines)
 }
 
+func TestRunForgetDryRunArgument(t *testing.T) {
+	bin := setupScriptBin(t, `echo "$@"`)
+	env := Env{Bin: bin}
+	lines, h := captureLines()
+	keepLast := 1
+	groups := []ForgetGroup{{
+		Tag:    "documents",
+		Policy: ForgetPolicy{KeepLast: &keepLast},
+	}}
+	if err := env.RunForget(context.Background(), groups, true, h); err != nil {
+		t.Fatalf("RunForget: %v", err)
+	}
+	for _, line := range *lines {
+		if strings.Contains(line, "forget --json --tag documents --keep-last 1 --dry-run") {
+			return
+		}
+	}
+	t.Fatalf("expected forget invocation with --dry-run; got: %v", *lines)
+}
+
 // --- B2: RunCheck ---
 
 func TestRunCheckLockSniff(t *testing.T) {

internal/server/http/jobs.go

@@ -65,10 +65,32 @@ func (s *Server) handleRunNow(w stdhttp.ResponseWriter, r *stdhttp.Request) {
 func (s *Server) dispatchJob(ctx context.Context, user *store.User,
 	hostID string, kind api.JobKind, args []string,
 ) (res runNowResponse, status int, code, msg string) {
-	return s.dispatchJobWithPayload(ctx, user, hostID, kind, nil, api.CommandRunPayload{
+	payload := api.CommandRunPayload{
 		Kind: kind,
 		Args: args,
-	})
+	}
+	if kind == api.JobForget {
+		if !validForgetArgs(args) {
+			return res, stdhttp.StatusBadRequest, "invalid_args",
+				"forget accepts no arguments other than --dry-run"
+		}
+		var ok bool
+		var err error
+		payload, ok, err = s.buildForgetPayloadForHost(ctx, hostID)
+		if err != nil {
+			return res, stdhttp.StatusInternalServerError, "internal", ""
+		}
+		if !ok {
+			return res, stdhttp.StatusUnprocessableEntity, "no_retention_policy",
+				"host has no source groups with a retention policy"
+		}
+		payload.Args = args
+	}
+	return s.dispatchJobWithPayload(ctx, user, hostID, kind, nil, payload)
+}
+
+func validForgetArgs(args []string) bool {
+	return len(args) == 0 || (len(args) == 1 && args[0] == "--dry-run")
 }
 
 // dispatchJobWithPayload is dispatchJob's variant that lets callers

internal/server/http/jobs_test.go

@@ -0,0 +1,106 @@
+package http
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	stdhttp "net/http"
+	"testing"
+	"time"
+
+	"github.com/oklog/ulid/v2"
+
+	"gitea.dcglab.co.uk/steve/restic-manager/internal/api"
+	"gitea.dcglab.co.uk/steve/restic-manager/internal/store"
+)
+
+func TestRunNowForgetShipsRetentionGroupsAndDryRun(t *testing.T) {
+	t.Parallel()
+	srv, ts, st := rawTestServer(t)
+	hostID, token := enrolHostForWS(t, srv, st, "manual-forget-host")
+	seedInitJob(t, st, hostID)
+
+	keepDaily := 7
+	if err := st.CreateSourceGroup(context.Background(), &store.SourceGroup{
+		ID:              ulid.Make().String(),
+		HostID:          hostID,
+		Name:            "documents",
+		Includes:        []string{"/home/documents"},
+		RetentionPolicy: store.RetentionPolicy{KeepDaily: &keepDaily},
+	}); err != nil {
+		t.Fatalf("create source group: %v", err)
+	}
+
+	c := agentDial(t, srv, ts, hostID, token)
+	sendHello(t, c, "manual-forget-host")
+	_ = drainUntil(t, c, api.MsgScheduleSet)
+
+	body, err := json.Marshal(runNowRequest{Kind: api.JobForget, Args: []string{"--dry-run"}})
+	if err != nil {
+		t.Fatalf("marshal request: %v", err)
+	}
+	req, err := stdhttp.NewRequest(stdhttp.MethodPost, ts.URL+"/api/hosts/"+hostID+"/jobs", bytes.NewReader(body))
+	if err != nil {
+		t.Fatalf("new request: %v", err)
+	}
+	req.Header.Set("Content-Type", "application/json")
+	req.AddCookie(loginAsAdmin(t, st))
+	res, err := stdhttp.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatalf("post run-now: %v", err)
+	}
+	defer res.Body.Close()
+	if res.StatusCode != stdhttp.StatusAccepted {
+		t.Fatalf("status: got %d, want %d", res.StatusCode, stdhttp.StatusAccepted)
+	}
+
+	got := readNextCommandRun(t, c, time.Now().Add(2*time.Second))
+	if got == nil {
+		t.Fatal("no command.run received")
+	}
+	if len(got.Args) != 1 || got.Args[0] != "--dry-run" {
+		t.Fatalf("Args: got %q, want [--dry-run]", got.Args)
+	}
+	if len(got.ForgetGroups) != 1 {
+		t.Fatalf("ForgetGroups: got %d, want 1", len(got.ForgetGroups))
+	}
+	group := got.ForgetGroups[0]
+	if group.Tag != "documents" || group.Policy.KeepDaily == nil || *group.Policy.KeepDaily != 7 {
+		t.Fatalf("ForgetGroups[0]: got %+v", group)
+	}
+}
+
+func TestRunNowForgetRejectsHostWithoutRetention(t *testing.T) {
+	t.Parallel()
+	srv, ts, st := rawTestServer(t)
+	hostID, token := enrolHostForWS(t, srv, st, "no-manual-retention-host")
+	seedInitJob(t, st, hostID)
+
+	c := agentDial(t, srv, ts, hostID, token)
+	sendHello(t, c, "no-manual-retention-host")
+	_ = drainUntil(t, c, api.MsgScheduleSet)
+
+	body := []byte(`{"kind":"forget","args":["--dry-run"]}`)
+	req, err := stdhttp.NewRequest(stdhttp.MethodPost, ts.URL+"/api/hosts/"+hostID+"/jobs", bytes.NewReader(body))
+	if err != nil {
+		t.Fatalf("new request: %v", err)
+	}
+	req.Header.Set("Content-Type", "application/json")
+	req.AddCookie(loginAsAdmin(t, st))
+	res, err := stdhttp.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatalf("post run-now: %v", err)
+	}
+	defer res.Body.Close()
+	if res.StatusCode != stdhttp.StatusUnprocessableEntity {
+		t.Fatalf("status: got %d, want %d", res.StatusCode, stdhttp.StatusUnprocessableEntity)
+	}
+
+	var jobs int
+	if err := st.DB().QueryRow(`SELECT COUNT(*) FROM jobs WHERE host_id = ? AND kind = 'forget'`, hostID).Scan(&jobs); err != nil {
+		t.Fatalf("count forget jobs: %v", err)
+	}
+	if jobs != 0 {
+		t.Fatalf("forget jobs: got %d, want 0", jobs)
+	}
+}

internal/server/http/maintenance_dispatch.go

@@ -37,7 +37,12 @@ func (s *Server) DispatchMaintenance(ctx context.Context, decisions []maintenanc
 		}
 		switch d.Kind {
 		case "forget":
-			payload, ok := s.buildForgetPayloadForHost(ctx, d.HostID)
+			payload, ok, err := s.buildForgetPayloadForHost(ctx, d.HostID)
+			if err != nil {
+				slog.Warn("maintenance: list source groups failed",
+					"host_id", d.HostID, "err", err)
+				continue
+			}
 			if !ok {
 				slog.Info("maintenance: forget skipped — no source groups with retention",
 					"host_id", d.HostID)
@@ -88,11 +93,10 @@ func (s *Server) DispatchMaintenance(ctx context.Context, decisions []maintenanc
 // that has a non-empty retention policy and builds a CommandRunPayload
 // with ForgetGroups populated. Returns ok=false if the host has no
 // such groups (the dispatcher then skips this kind).
-func (s *Server) buildForgetPayloadForHost(ctx context.Context, hostID string) (api.CommandRunPayload, bool) {
+func (s *Server) buildForgetPayloadForHost(ctx context.Context, hostID string) (api.CommandRunPayload, bool, error) {
 	groups, err := s.deps.Store.ListSourceGroupsByHost(ctx, hostID)
 	if err != nil {
-		slog.Warn("maintenance: list source groups failed", "host_id", hostID, "err", err)
-		return api.CommandRunPayload{}, false
+		return api.CommandRunPayload{}, false, err
 	}
 	fg := make([]api.ForgetGroup, 0, len(groups))
 	for _, g := range groups {
@@ -105,9 +109,9 @@ func (s *Server) buildForgetPayloadForHost(ctx context.Context, hostID string) (
 		})
 	}
 	if len(fg) == 0 {
-		return api.CommandRunPayload{}, false
+		return api.CommandRunPayload{}, false, nil
 	}
-	return api.CommandRunPayload{ForgetGroups: fg}, true
+	return api.CommandRunPayload{ForgetGroups: fg}, true, nil
 }
 
 func isEmptyRetention(p store.RetentionPolicy) bool {