Bug bash 2: 11 bugs in Stacks, Alerts, Docker, skins and the installer, fixed and verified (BUGS.md)

Stacks: deploy with every compose file (labels from inspect, not comma-joined ps); refuse a stale overwrite; empty list without Docker. Alerts: firing state survives a restart without re-announcing; vanished alerts are recorded as cleared with the reason; rule values validated; 200 rules. Docker: 404 for a removed container's logs. Skins: four palettes lifted to readable contrast. Phone: bigger toggles and swatches. Installer: deploy republishes; an older published build is never installed silently. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-19 15:59:51 -07:00
parent 9c4f9dfa9f
commit eddba4250b
10 changed files with 320 additions and 68 deletions
@@ -71,8 +71,18 @@ type alertSaved struct {
 	Channels []AlertChannel `json:"channels"`
 	Events   []AlertEvent   `json:"events"`
 	Seeded   bool           `json:"seeded"`
+	// Firing is what was firing when the hub last looked, so a restart (every deploy is one)
+	// carries on where it was and doesn't announce the same alerts all over again.
+	Firing map[string]alertKept `json:"firing,omitempty"`
 }

+type alertKept struct {
+	Since int64 `json:"since"`
+	Value any   `json:"value"`
+}
+
+const maxRules = 200
+
 type alertState struct {
 	pending time.Time
 	firing  bool
@@ -109,6 +119,9 @@ func newAlertEngine(s *Server) *alertEngine {
 	if !e.saved.Seeded {
 		e.saved.Rules, e.saved.Seeded = append([]AlertRule{}, defaultRules...), true
 	}
+	for key, k := range e.saved.Firing {
+		e.state[key] = &alertState{pending: time.UnixMilli(k.Since), firing: true, since: time.UnixMilli(k.Since), value: k.Value}
+	}
 	for i, r := range e.saved.Rules { // the first day's default watched "not running", which a container stopped on purpose also is
 		if r.ID == "container-down" && r.Sensor == "container.*.running" {
 			e.saved.Rules[i].Name, e.saved.Rules[i].Sensor, e.saved.Rules[i].Value = "Container down", "container.*.down", true
@@ -275,12 +288,47 @@ func (e *alertEngine) evaluate() {
 			}
 		}
 	}
-	for key := range e.state { // a sensor that is gone (a removed container, a deleted rule) takes its state with it
-		if !seen[key] {
-			delete(e.state, key)
+	byID := map[string]AlertRule{}
+	for _, r := range rules {
+		byID[r.ID] = r
+	}
+	for key, st := range e.state { // no longer watched: the rule was paused, deleted or narrowed, or the sensor is gone
+		if seen[key] {
+			continue
 		}
+		if st.firing { // it was on the Alerts tab and perhaps on a phone: say that it is over, and why
+			part := strings.SplitN(key, "|", 3)
+			r, known := byID[part[0]]
+			name, why := r.Name, "no longer watched"
+			if !known {
+				name, why = part[0], "rule deleted"
+			} else if !r.Enabled {
+				why = "rule paused"
+			}
+			if name == "" {
+				name = part[0]
+			}
+			events = append(events, AlertEvent{T: now.UnixMilli(), Kind: "resolved", Rule: name, RuleID: part[0], Machine: part[1], Sensor: part[2], Value: why, Severity: r.Severity})
+		}
+		delete(e.state, key)
 	}
 	e.last = now
+	kept := map[string]alertKept{}
+	for key, st := range e.state {
+		if st.firing {
+			kept[key] = alertKept{Since: st.since.UnixMilli(), Value: st.value}
+		}
+	}
+	changed := len(kept) != len(e.saved.Firing)
+	for k := range kept {
+		if _, ok := e.saved.Firing[k]; !ok {
+			changed = true
+		}
+	}
+	e.saved.Firing = kept
+	if changed && len(events) == 0 {
+		e.persist()
+	}
 	if len(events) > 0 {
 		e.saved.Events = append(e.saved.Events, events...)
 		if len(e.saved.Events) > maxEvents {
@@ -390,6 +438,10 @@ func (s *Server) alertsPut(w http.ResponseWriter, r *http.Request) {
 		jsonErr(w, 400, "bad request")
 		return
 	}
+	if body.Rules != nil && len(*body.Rules) > maxRules {
+		jsonErr(w, 400, fmt.Sprintf("at most %d rules: every one is checked on every machine every %d s", maxRules, int(alertEvery.Seconds())))
+		return
+	}
 	if body.Rules != nil {
 		ids := map[string]bool{}
 		for i := range *body.Rules {
@@ -408,6 +460,25 @@ func (s *Server) alertsPut(w http.ResponseWriter, r *http.Request) {
 				jsonErr(w, 400, fmt.Sprintf("rule %q: the comparison must be one of > >= < <= == !=", ru.Name))
 				return
 			}
+			switch val := ru.Value.(type) {
+			case float64, bool:
+			case string:
+				if len(val) > 200 {
+					jsonErr(w, 400, fmt.Sprintf("rule %q: the value is too long", ru.Name))
+					return
+				}
+				if ru.Op != "==" && ru.Op != "!=" { // "cpu.usage > ninety" can never be true, and would never say so
+					jsonErr(w, 400, fmt.Sprintf("rule %q: %s compares numbers, and %q is not one; text can only be == or !=", ru.Name, ru.Op, val))
+					return
+				}
+			default:
+				jsonErr(w, 400, fmt.Sprintf("rule %q: the value must be a number, true/false or text", ru.Name))
+				return
+			}
+			if _, isBool := ru.Value.(bool); isBool && ru.Op != "==" && ru.Op != "!=" {
+				jsonErr(w, 400, fmt.Sprintf("rule %q: true/false can only be == or !=", ru.Name))
+				return
+			}
 			if ru.Severity != "info" && ru.Severity != "crit" {
 				ru.Severity = "warn"
 			}
@@ -182,7 +182,11 @@ func (s *Server) dockerLogs(w http.ResponseWriter, r *http.Request) {
 	defer cancel()
 	out, err := dockerRun(ctx, append(args, id)...)
 	if err != nil {
-		jsonErr(w, 500, err.Error())
+		code := 500
+		if strings.Contains(strings.ToLower(err.Error()), "no such container") {
+			code = 404
+		}
+		jsonErr(w, code, err.Error())
 		return
 	}
 	type line struct {
@@ -16,6 +16,8 @@ package api
 import (
 	"bytes"
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -26,6 +28,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"sort"
+	"strconv"
 	"strings"
 	"time"

@@ -36,9 +39,11 @@ type stack struct {
 	Name       string   `json:"name"`
 	Containers []string `json:"containers"`
 	Running    int      `json:"running"`
-	File       string   `json:"file"`      // as the label has it
-	HostFile   string   `json:"host_file"` // where it is on this machine; "" when it isn't
-	Via        string   `json:"via"`       // container compose runs inside; "" = on the host
+	File       string   `json:"file"`       // as the label has it (the first of Files)
+	HostFile   string   `json:"host_file"`  // where it is on this machine; "" when it isn't
+	Files      []string `json:"files"`      // every file it was deployed from, in order: -f a -f b
+	HostFiles  []string `json:"host_files"` // the same, on this machine
+	Via        string   `json:"via"`        // container compose runs inside; "" = on the host
 	Editable   bool     `json:"editable"`
 	Note       string   `json:"note,omitempty"`
 	HasEnv     bool     `json:"has_env"`
@@ -86,45 +91,70 @@ func readable(p string) bool {
 }

 func (s *Server) findStacks(ctx context.Context) ([]stack, error) {
-	out, err := dockerRun(ctx, "ps", "-a", "--no-trunc", "--format", "{{json .}}")
+	// Labels come from inspect, not `ps`: ps joins every label with commas, and the label that
+	// matters here — the list of compose files — is itself a comma-separated list.
+	ids, err := dockerRun(ctx, "ps", "-aq")
 	if err != nil {
 		return nil, err
 	}
 	by := map[string]*stack{}
-	jsonLines(out, func(m map[string]any) {
-		labels := dstr(m, "Labels")
-		name := labelOf(labels, "com.docker.compose.project")
-		if name == "" {
-			return
+	if len(bytes.TrimSpace(ids)) > 0 {
+		const f = `{{.Name}}|{{.State.Status}}|{{index .Config.Labels "com.docker.compose.project"}}|{{index .Config.Labels "com.docker.compose.project.config_files"}}`
+		out, err := dockerRun(ctx, append([]string{"inspect", "-f", f}, strings.Fields(string(ids))...)...)
+		if err != nil {
+			return nil, err
 		}
-		st := by[name]
-		if st == nil {
-			st = &stack{Name: name, File: strings.Split(labelOf(labels, "com.docker.compose.project.config_files"), ",")[0]}
-			by[name] = st
+		for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") {
+			p := strings.SplitN(line, "|", 4)
+			if len(p) < 4 || p[2] == "" {
+				continue
+			}
+			st := by[p[2]]
+			if st == nil {
+				st = &stack{Name: p[2]}
+				if p[3] != "" {
+					st.Files = strings.Split(p[3], ",")
+					st.File = st.Files[0]
+				}
+				by[p[2]] = st
+			}
+			st.Containers = append(st.Containers, strings.TrimPrefix(p[0], "/"))
+			if p[1] == "running" {
+				st.Running++
+			}
 		}
-		st.Containers = append(st.Containers, strings.Split(dstr(m, "Names"), ",")[0])
-		if dstr(m, "State") == "running" {
-			st.Running++
-		}
-	})
+	}
 	maps := stackMounts(ctx)
 	stacks := []stack{}
 	for _, st := range by {
 		sort.Strings(st.Containers)
-		switch {
-		case st.File == "":
-			st.Note = "its containers don't say which file deployed them"
-		case readable(st.File):
-			st.HostFile = st.File
-		default:
+		// Every file must be found — a stack deployed from `-f compose.yaml -f override.yaml` and
+		// redeployed from the first alone would quietly lose everything the override set.
+		resolve := func(f string) (host, via string) {
+			if readable(f) {
+				return f, ""
+			}
 			for _, m := range maps {
-				if rest, ok := strings.CutPrefix(st.File, m.dest+"/"); ok && readable(filepath.Join(m.src, rest)) {
-					st.HostFile, st.Via = filepath.Join(m.src, rest), m.container
-					break
+				if rest, ok := strings.CutPrefix(f, m.dest+"/"); ok && readable(filepath.Join(m.src, rest)) {
+					return filepath.Join(m.src, rest), m.container
 				}
 			}
-			if st.HostFile == "" {
-				st.Note = "deployed from somewhere this machine can't see (" + st.File + "); edit it there"
+			return "", ""
+		}
+		if st.File == "" {
+			st.Note = "its containers don't say which file deployed them"
+		} else {
+			for i, f := range st.Files {
+				host, via := resolve(f)
+				if host == "" || (i > 0 && via != st.Via) {
+					st.HostFiles, st.Via = nil, ""
+					st.Note = "deployed from somewhere this machine can't see (" + f + "); edit it there"
+					break
+				}
+				st.HostFiles, st.Via = append(st.HostFiles, host), via
+			}
+			if len(st.HostFiles) > 0 {
+				st.HostFile = st.HostFiles[0]
 			}
 		}
 		if st.HostFile != "" {
@@ -153,6 +183,10 @@ func (s *Server) stackByName(ctx context.Context, name string) (stack, error) {
 func (s *Server) stacksList(w http.ResponseWriter, r *http.Request) {
 	ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second)
 	defer cancel()
+	if dockerTool() == "" { // no Docker is an empty answer, not a failure
+		jsonOK(w, 200, map[string]any{"stacks": []stack{}})
+		return
+	}
 	stacks, err := s.findStacks(ctx)
 	if err != nil {
 		jsonErr(w, 500, err.Error())
@@ -161,6 +195,26 @@ func (s *Server) stacksList(w http.ResponseWriter, r *http.Request) {
 	jsonOK(w, 200, map[string]any{"stacks": stacks})
 }

+func readAsRoot(ctx context.Context, path string) ([]byte, error) {
+	b, err := os.ReadFile(path)
+	if err != nil && priv.Available() {
+		full := priv.Wrap("cat", path)
+		b, err = exec.CommandContext(ctx, full[0], full[1:]...).Output()
+	}
+	return b, err
+}
+
+func sumOf(b []byte) string { h := sha256.Sum256(b); return hex.EncodeToString(h[:8]) }
+
+// fileIndex is which of the stack's files a request means: ?file=1 is the first override.
+func fileIndex(r *http.Request, st stack) (int, bool) {
+	i, err := strconv.Atoi(r.URL.Query().Get("file"))
+	if r.URL.Query().Get("file") == "" {
+		return 0, true
+	}
+	return i, err == nil && i >= 0 && i < len(st.HostFiles)
+}
+
 func (s *Server) stackFile(w http.ResponseWriter, r *http.Request) {
 	ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second)
 	defer cancel()
@@ -169,29 +223,48 @@ func (s *Server) stackFile(w http.ResponseWriter, r *http.Request) {
 		jsonErr(w, 404, "that stack's compose file isn't on this machine")
 		return
 	}
-	b, err := os.ReadFile(st.HostFile)
-	if err != nil && priv.Available() {
-		full := priv.Wrap("cat", st.HostFile)
-		b, err = exec.CommandContext(ctx, full[0], full[1:]...).Output()
+	i, ok := fileIndex(r, st)
+	if !ok {
+		jsonErr(w, 400, "no such file in this stack")
+		return
 	}
+	b, err := readAsRoot(ctx, st.HostFiles[i])
 	if err != nil {
 		jsonErr(w, 500, err.Error())
 		return
 	}
-	jsonOK(w, 200, map[string]any{"stack": st, "text": string(b)})
+	// sum is what the editor hands back when it saves, so a file changed meanwhile is noticed
+	jsonOK(w, 200, map[string]any{"stack": st, "text": string(b), "sum": sumOf(b), "index": i})
 }

 // composeCmd is `docker compose …` for this stack, run where its file makes sense.
-func composeCmd(ctx context.Context, st stack, file string, args ...string) *exec.Cmd {
+// files are given as compose will see them: the label's paths inside a container, host paths otherwise.
+func composeCmd(ctx context.Context, st stack, files []string, args ...string) *exec.Cmd {
 	tool := dockerTool()
-	if st.Via != "" {
-		return exec.CommandContext(ctx, tool, append([]string{"exec", st.Via, "docker", "compose", "-p", st.Name, "-f", file}, args...)...)
+	base := []string{"compose", "-p", st.Name}
+	for _, f := range files {
+		base = append(base, "-f", f)
 	}
-	cmd := exec.CommandContext(ctx, tool, append([]string{"compose", "-p", st.Name, "-f", file}, args...)...)
-	cmd.Dir = filepath.Dir(file)
+	if st.Via != "" {
+		return exec.CommandContext(ctx, tool, append(append([]string{"exec", st.Via, "docker"}, base...), args...)...)
+	}
+	cmd := exec.CommandContext(ctx, tool, append(base, args...)...)
+	cmd.Dir = filepath.Dir(files[0])
 	return cmd
 }

+// seenFiles is the stack's file list as compose runs it, with file i replaced by a candidate.
+func seenFiles(st stack, i int, candidate string) []string {
+	out := append([]string{}, st.HostFiles...)
+	if st.Via != "" {
+		out = append([]string{}, st.Files...)
+	}
+	if i >= 0 {
+		out[i] = candidate
+	}
+	return out
+}
+
 // stackSave validates the new text with compose itself, keeps the old file beside it as
 // <file>.hamlab-bak, and writes. It does not deploy: that is its own button.
 func (s *Server) stackSave(w http.ResponseWriter, r *http.Request) {
@@ -204,13 +277,24 @@ func (s *Server) stackSave(w http.ResponseWriter, r *http.Request) {
 	}
 	var body struct {
 		Text string `json:"text"`
+		Sum  string `json:"sum"` // the sum the editor was given when it opened the file
 	}
 	if err := json.NewDecoder(io.LimitReader(r.Body, 1<<20)).Decode(&body); err != nil || strings.TrimSpace(body.Text) == "" {
 		jsonErr(w, 400, "the file can't be empty")
 		return
 	}
+	i, ok := fileIndex(r, st)
+	if !ok {
+		jsonErr(w, 400, "no such file in this stack")
+		return
+	}
+	target := st.HostFiles[i]
+	if now, err := readAsRoot(ctx, target); err == nil && body.Sum != "" && sumOf(now) != body.Sum {
+		jsonErr(w, 409, "the file has changed on disk since you opened it (Dockhand, or another editor). Reload it to see the change; your text is still in the box.")
+		return
+	}
 	// Validate a candidate beside the real file, so `.env` and relative paths resolve as they will.
-	cand := st.HostFile + ".hamlab-new"
+	cand := target + ".hamlab-new"
 	err = s.fsLocal.Write(ctx, cand, strings.NewReader(body.Text)) // ours to write, or root's
 	if err != nil {
 		err = s.fsSudo.Write(ctx, cand, strings.NewReader(body.Text))
@@ -221,14 +305,14 @@ func (s *Server) stackSave(w http.ResponseWriter, r *http.Request) {
 	}
 	inner := cand
 	if st.Via != "" {
-		inner = st.File + ".hamlab-new"
+		inner = st.Files[i] + ".hamlab-new"
 	}
-	if msg, err := composeCmd(ctx, st, inner, "config", "-q").CombinedOutput(); err != nil {
+	if msg, err := composeCmd(ctx, st, seenFiles(st, i, inner), "config", "-q").CombinedOutput(); err != nil {
 		_ = s.fsSudo.Delete(ctx, cand, false)
 		jsonErr(w, 400, "compose rejects it: "+strings.TrimSpace(string(msg)))
 		return
 	}
-	for _, mv := range [][]string{{"cp", "-p", st.HostFile, st.HostFile + ".hamlab-bak"}, {"mv", cand, st.HostFile}} {
+	for _, mv := range [][]string{{"cp", "-p", target, target + ".hamlab-bak"}, {"mv", cand, target}} {
 		full := priv.Wrap(mv...)
 		if msg, err := exec.CommandContext(ctx, full[0], full[1:]...).CombinedOutput(); err != nil {
 			jsonErr(w, 500, strings.TrimSpace(string(msg)))
@@ -249,16 +333,18 @@ func (s *Server) stackAction(w http.ResponseWriter, r *http.Request) {
 		jsonErr(w, 404, "that stack's compose file isn't on this machine")
 		return
 	}
-	file := st.HostFile
-	if st.Via != "" {
-		file = st.File
+	files := seenFiles(st, -1, "")
+	var fl []string
+	for _, f := range files {
+		fl = append(fl, "-f", f)
 	}
+	flags := priv.Quote(append([]string{"-p", st.Name}, fl...))
 	var script string
 	base := func(args string) string {
 		if st.Via != "" {
-			return fmt.Sprintf("%s exec %s docker compose -p %s -f %s %s", dockerTool(), priv.Quote([]string{st.Via}), priv.Quote([]string{st.Name}), priv.Quote([]string{file}), args)
+			return fmt.Sprintf("%s exec %s docker compose %s %s", dockerTool(), priv.Quote([]string{st.Via}), flags, args)
 		}
-		return fmt.Sprintf("cd %s && %s compose -p %s -f %s %s", priv.Quote([]string{filepath.Dir(file)}), dockerTool(), priv.Quote([]string{st.Name}), priv.Quote([]string{file}), args)
+		return fmt.Sprintf("cd %s && %s compose %s %s", priv.Quote([]string{filepath.Dir(files[0])}), dockerTool(), flags, args)
 	}
 	switch r.PathValue("action") {
 	case "deploy":