A sweep of the whole console: status codes, a data race, atomic writes, dead code

From the race detector, staticcheck, a garbage-in battery over every safe route and a
headless pass through every tab on two machines. Fixed:
- docker inspect/health of a container that isn't there answered 500; now 404, and a
  daemon that is down 503 (dockerErr, with a test).
- /api/virt/instances/{name} on a machine without Incus answered 502 "incus not found";
  it now says Incus isn't ready, like its siblings.
- Deleting a ZFS snapshot with a bad name answered 500; a bad name is 400, a snapshot
  that doesn't exist 404.
- The task list read a task's Running flag without the task's lock while the waiter
  goroutine cleared it: a data race, now read under the lock.
- The Virtualization export config and run record were written in place; a crash
  mid-write would have left a half file. Atomic now, like the config and state files.
- An unused function in kvm.go, one in incus/subnet.go, an unused field in files/move.go
  and a capitalised error string, from staticcheck.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

internal/api/docker.go

@@ -343,6 +343,21 @@ func (s *Server) dockerLogs(w http.ResponseWriter, r *http.Request) {
 }
 
 // dockerInspect is the parts of `docker inspect` worth reading on a phone.
+// dockerErr answers for a docker command that failed: a thing that isn't there is a 404,
+// the daemon being away a 503, anything else a 500.
+func dockerErr(w http.ResponseWriter, err error) {
+	msg := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(err.Error()), "error: "))
+	low := strings.ToLower(msg)
+	switch {
+	case strings.Contains(low, "no such object") || strings.Contains(low, "no such container") || strings.Contains(low, "not found"):
+		jsonErr(w, 404, msg)
+	case strings.Contains(low, "cannot connect to the docker daemon") || strings.Contains(low, "is the docker daemon running"):
+		jsonErr(w, 503, msg)
+	default:
+		jsonErr(w, 500, msg)
+	}
+}
+
 func (s *Server) dockerInspect(w http.ResponseWriter, r *http.Request) {
 	id := r.PathValue("id")
 	if !dockerRef.MatchString(id) {
@@ -352,9 +367,13 @@ func (s *Server) dockerInspect(w http.ResponseWriter, r *http.Request) {
 	ctx, cancel := context.WithTimeout(r.Context(), 15*time.Second)
 	defer cancel()
 	out, err := dockerRun(ctx, "inspect", id)
+	if err != nil {
+		dockerErr(w, err)
+		return
+	}
 	var all []map[string]any
-	if err != nil || json.Unmarshal(out, &all) != nil || len(all) == 0 {
+	if json.Unmarshal(out, &all) != nil || len(all) == 0 {
-		jsonErr(w, 500, fmt.Sprint("inspect failed: ", err))
+		jsonErr(w, 500, "inspect: unreadable answer")
 		return
 	}
 	c := all[0]
@@ -486,7 +505,7 @@ func (s *Server) dockerHealth(w http.ResponseWriter, r *http.Request) {
 	defer cancel()
 	out, err := dockerRun(ctx, "inspect", "--format", "{{json .State}}", id)
 	if err != nil {
-		jsonErr(w, 500, err.Error())
+		dockerErr(w, err)
 		return
 	}
 	var st struct {

internal/api/files_test.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"bytes"
+	"errors"
 	"net/http/httptest"
 	"net/url"
 	"os"
@@ -102,3 +103,21 @@ func TestFilesReadDanglingLink(t *testing.T) {
 		t.Fatalf("got %d: %s", w.Code, w.Body.String())
 	}
 }
+
+// dockerErr tells a thing that isn't there from a daemon that isn't up from anything else.
+func TestDockerErr(t *testing.T) {
+	for _, c := range []struct {
+		msg  string
+		want int
+	}{
+		{"error: No such object: zzz", 404}, {"Error response from daemon: No such container: x", 404},
+		{"Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?", 503},
+		{"something else entirely", 500},
+	} {
+		w := httptest.NewRecorder()
+		dockerErr(w, errors.New(c.msg))
+		if w.Code != c.want {
+			t.Errorf("%q → %d, want %d", c.msg, w.Code, c.want)
+		}
+	}
+}

internal/api/kvm.go

@@ -338,20 +338,6 @@ func (s *Server) listKVMs(w http.ResponseWriter, _ *http.Request) {
 	jsonOK(w, 200, out)
 }
 
-// kvmForMachine is the KVM attached to a machine, "" being the hub itself.
-func (s *Server) kvmForMachine(machine string) (config.KVM, bool) {
-	var found config.KVM
-	ok := false
-	s.cfg.View(func(c *config.Config) {
-		for _, k := range c.KVMs {
-			if k.Machine == machine {
-				found, ok = k, true
-			}
-		}
-	})
-	return found, ok
-}
-
 func (s *Server) addKVM(w http.ResponseWriter, r *http.Request) {
 	var body struct{ Name, URL, Machine string }
 	if err := json.NewDecoder(io.LimitReader(r.Body, 4096)).Decode(&body); err != nil {

internal/api/virt.go

@@ -286,6 +286,9 @@ func (s *Server) virtInstance(w http.ResponseWriter, r *http.Request) {
 		jsonErr(w, 400, "bad name")
 		return
 	}
+	if !s.virtReady(w, r) {
+		return
+	}
 	list, err := s.virt.Instances(r.Context())
 	if err != nil {
 		jsonErr(w, 502, err.Error())

internal/api/zfs.go

@@ -265,7 +265,15 @@ func (s *Server) zfsSnapshotDelete(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if err := s.storage.DestroySnapshot(r.Context(), body.Name, body.Recursive); err != nil {
-		jsonErr(w, 500, err.Error())
+		msg := err.Error()
+		switch {
+		case msg == "bad snapshot name":
+			jsonErr(w, 400, msg)
+		case strings.Contains(msg, "does not exist"):
+			jsonErr(w, 404, msg)
+		default:
+			jsonErr(w, 500, msg)
+		}
 		return
 	}
 	log.Printf("zfs snapshot %s destroyed by %s%s", body.Name, clientIP(r), map[bool]string{true: " (recursive)", false: ""}[body.Recursive])

internal/files/move.go

@@ -73,12 +73,11 @@ func moveByCopy(ctx context.Context, from, to string, w io.Writer) error {
 }
 
 type progress struct {
-	w           io.Writer
+	w          io.Writer
-	files       int64
+	files      int64
-	bytes       int64
+	bytes      int64
-	shownFiles  int64
+	shownFiles int64
-	shownBytes  int64
+	shownBytes int64
-	lastPrinted bool
 }
 
 func (p *progress) add(files, bytes int64) {

internal/incus/export.go

@@ -10,6 +10,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"hamlab/internal/store"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -91,7 +92,7 @@ func (m *Manager) SaveExportConfig(c ExportConfig) error {
 	}
 	c.Updated = time.Now()
 	b, _ := json.MarshalIndent(c, "", "  ")
-	return os.WriteFile(m.exportConfigPath(), b, 0o644)
+	return store.WriteFileAtomic(m.exportConfigPath(), b, 0o644)
 }
 
 type ExportRun struct {
@@ -226,6 +227,6 @@ func (m *Manager) RunExports(ctx context.Context, names []string, w func(string)
 	}
 	run.Seconds = time.Since(start).Seconds()
 	b, _ := json.MarshalIndent(run, "", "  ")
-	_ = os.WriteFile(m.exportRunPath(), b, 0o644)
+	_ = store.WriteFileAtomic(m.exportRunPath(), b, 0o644)
 	return run
 }

internal/incus/subnet.go

@@ -14,7 +14,6 @@ import (
 	"net/netip"
 	"os"
 	"os/exec"
-	"strings"
 	"time"
 )
 
@@ -125,6 +124,3 @@ func dockerPools() []string {
 	}
 	return []string{"172.17.0.0/12", "192.168.0.0/16"}
 }
-
-// String form used in the layout file and on the page.
-func prefixString(p netip.Prefix) string { return strings.TrimSpace(p.Masked().String()) }

internal/incus/vga.go

@@ -130,7 +130,7 @@ func (m *Manager) openVGA(ctx context.Context, client *http.Client, name string,
 	resp, err := client.Do(req)
 	if err != nil {
 		if isPermission(err) {
-			return nil, errors.New("Ham Lab's account can't reach Incus's socket; Finish setup on the Virtualization tab puts it in incus-admin")
+			return nil, errors.New("this account can't reach Incus's socket; Finish setup on the Virtualization tab puts Ham Lab in incus-admin")
 		}
 		return nil, err
 	}

internal/tasks/tasks.go

@@ -146,7 +146,7 @@ func (m *Manager) prune() {
 	}
 	var finished []*Task
 	for _, t := range m.tasks {
-		if !t.Running {
+		if !t.running() {
 			finished = append(finished, t)
 		}
 	}
@@ -184,13 +184,20 @@ func (m *Manager) RunningOf(kind string) (*Task, bool) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	for _, t := range m.tasks {
-		if t.Kind == kind && t.Running {
+		if t.Kind == kind && t.running() {
 			return t, true
 		}
 	}
 	return nil, false
 }
 
+// running reads the flag the waiter goroutine clears, under the task's own lock.
+func (t *Task) running() bool {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	return t.Running
+}
+
 func (t *Task) Snapshot(withOutput bool) Snapshot {
 	t.mu.Lock()
 	defer t.mu.Unlock()