R-569: stack-lifecycle handlers pick their status by error KIND, not English words

stop/start/restart/update, remove and delete matched "protected", "not found", "not deployed",
"still running", "not orphaned" in err.Error(). New sentinels in internal/stacks
(stack_errors.go) carried by the producers in manager.go/delete.go via util.KindErrorf (message
bytes unchanged); api.stackOpStatusFor maps them. Tests: reworded-message table per family,
wiring check over all three handlers, producers keep kind + words.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
This commit is contained in:
2026-10-05 21:36:54 +02:00
parent 9271f33359
commit d15ad105be
7 changed files with 203 additions and 47 deletions
@@ -0,0 +1,80 @@
package api
import (
"errors"
"fmt"
"net/http"
"os"
"strings"
"testing"
"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
"gitea.dooplex.hu/admin/felhom-controller/internal/util"
)
// R-569 — the stop/start/restart/update, remove and delete handlers pick their status by the
// refusal's KIND. Every row passes a REWORDED message: the words the handlers used to match
// ("protected", "not found", "not deployed", "still running", "not orphaned") are gone, only the
// kind remains. RED-PROOF (REPORT): restore any strings.Contains chain and its family's reworded rows
// answer 500.
func TestR569_StackOpStatus_SurvivesRewording(t *testing.T) {
cases := []struct {
family, name string
err error
want int
}{
// action family (start/stop/restart/update)
{"action", "unknown app, reworded", util.KindErrorf(stacks.ErrStackNotFound, "no app called %q", "x"), http.StatusNotFound},
{"action", "unknown app, wrapped by the caller", fmt.Errorf("restarting: %w",
util.KindErrorf(stacks.ErrStackNotFound, "no app called %q", "x")), http.StatusNotFound},
{"action", "protected, reworded", util.KindErrorf(stacks.ErrProtectedStack, "%q is infrastructure", "traefik"), http.StatusForbidden},
{"action", "update refusal", &stacks.UpdateRefusal{Reason: "busy", Message: "foglalt"}, http.StatusConflict},
{"action", "update refusal: unknown app", &stacks.UpdateRefusal{Reason: "not_found", Message: "nincs ilyen"}, http.StatusNotFound},
// remove family
{"remove", "not deployed, reworded", util.KindErrorf(stacks.ErrNotDeployed, "%q has nothing installed", "x"), http.StatusConflict},
{"remove", "still running, reworded", util.KindErrorf(stacks.ErrStillRunning, "%q is up — stop it first", "x"), http.StatusConflict},
{"remove", "protected, reworded", util.KindErrorf(stacks.ErrProtectedStack, "cannot take %q away", "traefik"), http.StatusForbidden},
// delete family
{"delete", "not orphaned, reworded", util.KindErrorf(stacks.ErrNotOrphaned, "%q still belongs to the catalog", "x"), http.StatusConflict},
{"delete", "still running, reworded", util.KindErrorf(stacks.ErrStillRunning, "%q is up", "x"), http.StatusConflict},
{"delete", "unknown, reworded", util.KindErrorf(stacks.ErrStackNotFound, "nothing named %q", "x"), http.StatusNotFound},
// the negative half: the old WORDS without a kind are not a refusal
{"any", "text that merely contains the old words", errors.New("docker: image not found; protected; still running"), http.StatusInternalServerError},
{"any", "anything else", errors.New("compose exploded"), http.StatusInternalServerError},
}
for _, c := range cases {
if got := stackOpStatusFor(c.err); got != c.want {
t.Errorf("%s / %s: stackOpStatusFor(%q) = %d, want %d", c.family, c.name, c.err.Error(), got, c.want)
}
}
}
// The seam must be WIRED in all three handlers, and none may decide by reading the error text again.
func TestR569_HandlersUseTheKind(t *testing.T) {
src, err := os.ReadFile("router.go")
if err != nil {
t.Fatal(err)
}
body := string(src)
for _, fn := range []string{"func (r *Router) actionStack(", "func (r *Router) removeStack(", "func (r *Router) deleteStack("} {
i := strings.Index(body, fn)
if i < 0 {
t.Fatalf("%s not found — this test no longer reads what it thinks it reads", fn)
}
h := body[i:]
if j := strings.Index(h[10:], "\nfunc "); j > 0 {
h = h[:j+10]
}
if !strings.Contains(h, "stackOpStatusFor(err)") {
t.Errorf("%s no longer asks stackOpStatusFor for the status code", fn)
}
for _, line := range strings.Split(h, "\n") {
if strings.HasPrefix(strings.TrimSpace(line), "//") {
continue
}
if strings.Contains(line, "strings.Contains(err.Error()") {
t.Errorf("%s decides by reading the error text again (R-569): %s", fn, strings.TrimSpace(line))
}
}
}
}
+33 -31
View File
@@ -623,6 +623,36 @@ func desiredStateForAction(action string) (string, bool) {
}
}
// stackOpStatusFor maps a stack-lifecycle refusal (stop/start/restart/update, remove, delete) to its
// HTTP status by the refusal's KIND (R-569) — the R-553 rule one handler family over.
//
// Until R-569 these three handlers matched "protected", "not found", "not deployed", "still running"
// and "not orphaned" in err.Error(). Those strings are internal English, so localisation never moved
// them; a reworded internal message would have, silently turning a 404/403/409 into a 500. The kinds
// come from internal/stacks (stack_errors.go); the messages are unchanged.
//
// One function serves the three families because their kinds are disjoint: an action never returns
// ErrNotOrphaned, a remove never returns an UpdateRefusal. Pinned by
// TestR569_StackOpStatus_SurvivesRewording and TestR569_HandlersUseTheKind.
func stackOpStatusFor(err error) int {
var ref *stacks.UpdateRefusal
switch {
case errors.Is(err, stacks.ErrStackNotFound):
return http.StatusNotFound
case errors.As(err, &ref):
if ref.Reason == "not_found" {
return http.StatusNotFound
}
return http.StatusConflict
case errors.Is(err, stacks.ErrProtectedStack):
return http.StatusForbidden
case errors.Is(err, stacks.ErrNotDeployed), errors.Is(err, stacks.ErrStillRunning),
errors.Is(err, stacks.ErrNotOrphaned):
return http.StatusConflict
}
return http.StatusInternalServerError
}
func (r *Router) actionStack(w http.ResponseWriter, req *http.Request, action, name string) {
r.logger.Printf("[INFO] [api] %s requested for stack: %s", action, name)
r.dbg("actionStack: action=%s name=%s", action, name)
@@ -760,17 +790,7 @@ func (r *Router) actionStack(w http.ResponseWriter, req *http.Request, action, n
if err != nil {
r.logger.Printf("[ERROR] [api] %s failed for %s: %v", action, name, err)
status := http.StatusInternalServerError
var ref *stacks.UpdateRefusal
if errors.As(err, &ref) {
status = http.StatusConflict
}
if strings.Contains(err.Error(), "protected") {
status = http.StatusForbidden
}
if strings.Contains(err.Error(), "not found") {
status = http.StatusNotFound
}
status := stackOpStatusFor(err) // R-569: by kind, never by the error's words
writeJSON(w, status, apiResponse{OK: false, Error: r.errText(req, err)})
return
}
@@ -1052,16 +1072,7 @@ func (r *Router) removeStack(w http.ResponseWriter, req *http.Request, name stri
writeJSON(w, http.StatusConflict, apiResponse{OK: false, Error: r.errText(req, err)})
return
}
status := http.StatusInternalServerError
if strings.Contains(err.Error(), "protected") {
status = http.StatusForbidden
}
if strings.Contains(err.Error(), "not found") {
status = http.StatusNotFound
}
if strings.Contains(err.Error(), "not deployed") || strings.Contains(err.Error(), "still running") {
status = http.StatusConflict
}
status := stackOpStatusFor(err) // R-569: by kind, never by the error's words
writeJSON(w, status, apiResponse{OK: false, Error: r.errText(req, err)})
return
}
@@ -1137,16 +1148,7 @@ func (r *Router) deleteStack(w http.ResponseWriter, req *http.Request, name stri
writeJSON(w, http.StatusConflict, apiResponse{OK: false, Error: refused.Message})
return
}
status := http.StatusInternalServerError
if strings.Contains(err.Error(), "protected") {
status = http.StatusForbidden
}
if strings.Contains(err.Error(), "not found") {
status = http.StatusNotFound
}
if strings.Contains(err.Error(), "not orphaned") || strings.Contains(err.Error(), "still running") {
status = http.StatusConflict
}
status := stackOpStatusFor(err) // R-569: by kind, never by the error's words
writeJSON(w, status, apiResponse{OK: false, Error: r.errText(req, err)})
return
}
+10 -10
View File
@@ -269,12 +269,12 @@ func (m *Manager) DeleteStack(name string, removeHDDData bool) (*DeleteResponse,
// Safety: never delete protected stacks
if m.cfg.IsProtectedStack(name) {
return nil, fmt.Errorf("stack %q is protected and cannot be deleted", name)
return nil, util.KindErrorf(ErrProtectedStack, "stack %q is protected and cannot be deleted", name)
}
stack, ok := m.GetStack(name)
if !ok {
return nil, fmt.Errorf("stack %q not found", name)
return nil, util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
if m.isDebug() {
@@ -284,7 +284,7 @@ func (m *Manager) DeleteStack(name string, removeHDDData bool) (*DeleteResponse,
// Must be orphaned
if !stack.Orphaned {
return nil, fmt.Errorf("stack %q is not orphaned — only orphaned stacks can be deleted", name)
return nil, util.KindErrorf(ErrNotOrphaned, "stack %q is not orphaned — only orphaned stacks can be deleted", name)
}
// Must not be deploying (H2 fix)
@@ -296,7 +296,7 @@ func (m *Manager) DeleteStack(name string, removeHDDData bool) (*DeleteResponse,
// StateDegraded (R-51) counts as running here: a degraded stack still has LIVE containers, and
// deleting its directory out from under them would leave orphans behind.
if stack.State == StateRunning || stack.State == StateStarting || stack.State == StateRestarting || stack.State == StateDegraded {
return nil, fmt.Errorf("stack %q is still running — stop it first before deleting", name)
return nil, util.KindErrorf(ErrStillRunning, "stack %q is still running — stop it first before deleting", name)
}
stackDir := filepath.Dir(stack.ComposePath)
@@ -417,7 +417,7 @@ func (m *Manager) DeleteStack(name string, removeHDDData bool) (*DeleteResponse,
func (m *Manager) GetStackHDDData(name string) (*HDDDataResponse, error) {
stack, ok := m.GetStack(name)
if !ok {
return nil, fmt.Errorf("stack %q not found", name)
return nil, util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
// R-442: the app's own recorded HDD_PATH, not the global config (which no box sets).
@@ -577,12 +577,12 @@ func (m *Manager) RemoveStack(name string, removeHDDData bool, backupPathsToRemo
// Safety: never remove protected stacks
if m.cfg.IsProtectedStack(name) {
return nil, fmt.Errorf("stack %q is protected and cannot be removed", name)
return nil, util.KindErrorf(ErrProtectedStack, "stack %q is protected and cannot be removed", name)
}
stack, ok := m.GetStack(name)
if !ok {
return nil, fmt.Errorf("stack %q not found", name)
return nil, util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
if m.isDebug() {
@@ -601,7 +601,7 @@ func (m *Manager) RemoveStack(name string, removeHDDData bool, backupPathsToRemo
if !stack.Deployed {
half, why := m.halfStateEvidence(name, stack)
if !half {
return nil, fmt.Errorf("stack %q is not deployed", name)
return nil, util.KindErrorf(ErrNotDeployed, "stack %q is not deployed", name)
}
m.logger.Printf("[WARN] [stacks] RemoveStack %s: deployed=false but %s — removing what exists (R-634)", name, why)
}
@@ -632,7 +632,7 @@ func (m *Manager) RemoveStack(name string, removeHDDData bool, backupPathsToRemo
// StateDegraded (R-51) counts as running here: a degraded stack still has LIVE containers, and
// deleting its directory out from under them would leave orphans behind.
if stack.State == StateRunning || stack.State == StateStarting || stack.State == StateRestarting || stack.State == StateDegraded {
return nil, fmt.Errorf("stack %q is still running — stop it first before removing", name)
return nil, util.KindErrorf(ErrStillRunning, "stack %q is still running — stop it first before removing", name)
}
stackDir := filepath.Dir(stack.ComposePath)
@@ -830,7 +830,7 @@ func (m *Manager) RemoveStack(name string, removeHDDData bool, backupPathsToRemo
func (m *Manager) GetStackBackupData(name string, drivePath string, mirrorDirs []string) (*BackupDataResponse, error) {
_, ok := m.GetStack(name)
if !ok {
return nil, fmt.Errorf("stack %q not found", name)
return nil, util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
resp := &BackupDataResponse{
+7 -6
View File
@@ -21,6 +21,7 @@ import (
"gitea.dooplex.hu/admin/felhom-controller/internal/crypto"
"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
"gitea.dooplex.hu/admin/felhom-controller/internal/system"
"gitea.dooplex.hu/admin/felhom-controller/internal/util"
)
// ContainerState represents the current state of a container.
@@ -1275,7 +1276,7 @@ func deepCopyStack(s *Stack) Stack {
func (m *Manager) StartStack(name string) error {
stack, ok := m.GetStack(name)
if !ok {
return fmt.Errorf("stack %q not found", name)
return util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
if stack.Deploying {
// R-634: a second `compose up -d` beside the deploy's own is the race that left apps running
@@ -1338,7 +1339,7 @@ func (m *Manager) StartStackServices(name string, services []string) error {
}
stack, ok := m.GetStack(name)
if !ok {
return fmt.Errorf("stack %q not found", name)
return util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
m.logger.Printf("[INFO] [stacks] Starting stack %s services only: %v", name, services)
@@ -1358,12 +1359,12 @@ func (m *Manager) StartStackServices(name string, services []string) error {
func (m *Manager) StopStack(name string) error {
if m.cfg.IsProtectedStack(name) {
return fmt.Errorf("stack %q is protected and cannot be stopped", name)
return util.KindErrorf(ErrProtectedStack, "stack %q is protected and cannot be stopped", name)
}
stack, ok := m.GetStack(name)
if !ok {
return fmt.Errorf("stack %q not found", name)
return util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
if stack.Deploying {
// R-634, the backstop for EVERY caller (backup, quiesce, restore, export, storage, the Stop
@@ -1393,7 +1394,7 @@ func (m *Manager) StopStack(name string) error {
func (m *Manager) RestartStack(name string) error {
stack, ok := m.GetStack(name)
if !ok {
return fmt.Errorf("stack %q not found", name)
return util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
if m.isDebug() {
@@ -1437,7 +1438,7 @@ func (m *Manager) RestartStack(name string) error {
func (m *Manager) GetLogs(name string, lines int) (string, error) {
stack, ok := m.GetStack(name)
if !ok {
return "", fmt.Errorf("stack %q not found", name)
return "", util.KindErrorf(ErrStackNotFound, "stack %q not found", name)
}
if lines <= 0 {
@@ -0,0 +1,51 @@
package stacks
import (
"errors"
"testing"
)
// R-569 — the stack-lifecycle refusals the API maps to 404/403/409 carry their KIND, and their
// messages are byte-for-byte what they were. Every case below refuses BEFORE anything is executed
// (no compose, no docker): unknown name, protected name, or a scanned app that is not orphaned.
//
// RED-PROOF (REPORT): put any one producer back to plain fmt.Errorf and its errors.Is row fails while
// its message row still passes — the silent state the API's old text match lived in.
func TestR569_StackProducersCarryKindAndKeepTheirWords(t *testing.T) {
m := r553Manager(t, "display_name: R569\n")
cases := []struct {
name string
call func() error
kind error
wantMsg string
}{
{"start unknown", func() error { return m.StartStack("nope") }, ErrStackNotFound, `stack "nope" not found`},
{"stop unknown", func() error { return m.StopStack("nope") }, ErrStackNotFound, `stack "nope" not found`},
{"restart unknown", func() error { return m.RestartStack("nope") }, ErrStackNotFound, `stack "nope" not found`},
{"stop protected", func() error { return m.StopStack("samba") }, ErrProtectedStack,
`stack "samba" is protected and cannot be stopped`},
{"remove unknown", func() error { _, err := m.RemoveStack("nope", false, nil); return err }, ErrStackNotFound,
`stack "nope" not found`},
{"remove protected", func() error { _, err := m.RemoveStack("samba", false, nil); return err }, ErrProtectedStack,
`stack "samba" is protected and cannot be removed`},
{"delete unknown", func() error { _, err := m.DeleteStack("nope", false); return err }, ErrStackNotFound,
`stack "nope" not found`},
{"delete protected", func() error { _, err := m.DeleteStack("samba", false); return err }, ErrProtectedStack,
`stack "samba" is protected and cannot be deleted`},
{"delete not orphaned", func() error { _, err := m.DeleteStack("r553app", false); return err }, ErrNotOrphaned,
`stack "r553app" is not orphaned — only orphaned stacks can be deleted`},
}
for _, c := range cases {
err := c.call()
if err == nil {
t.Errorf("%s: no refusal", c.name)
continue
}
if !errors.Is(err, c.kind) {
t.Errorf("%s: errors.Is(err, %v) = false — the API would answer 500 (err=%q)", c.name, c.kind, err.Error())
}
if err.Error() != c.wantMsg {
t.Errorf("%s: message moved:\n got %q\n want %q", c.name, err.Error(), c.wantMsg)
}
}
}
@@ -0,0 +1,21 @@
package stacks
import "errors"
// R-569 — the stack-lifecycle refusals carry a KIND, so the API's stop/start/restart/update, remove
// and delete handlers pick their status code with errors.Is instead of matching "protected",
// "not found", "not deployed", "still running" or "not orphaned" in the error text. The messages are
// unchanged byte for byte (util.KindErrorf); a reworded message no longer moves a status code.
// Pinned by TestR569_StackOpStatus_SurvivesRewording (internal/api).
var (
// ErrStackNotFound — no stack by that name (API: 404).
ErrStackNotFound = errors.New("stack not found")
// ErrProtectedStack — an infrastructure stack the household may not stop/remove/delete (API: 403).
ErrProtectedStack = errors.New("stack is protected")
// ErrNotDeployed — remove asked of a stack with nothing deployed (API: 409).
ErrNotDeployed = errors.New("stack is not deployed")
// ErrStillRunning — remove/delete asked of a running stack (API: 409).
ErrStillRunning = errors.New("stack is still running")
// ErrNotOrphaned — delete asked of a stack that is not orphaned (API: 409).
ErrNotOrphaned = errors.New("stack is not orphaned")
)