diff --git a/CHANGELOG.md b/CHANGELOG.md index d821aef..9df06eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,18 @@ +## v0.262.1 — the busy refusal is a CONFLICT, not a server error (2026-09-22, R-633) + +**MinAgent: 0.131.0** (unchanged). No new strings. + +**Caught by the live proof, not by a test.** v0.262.0's remove-busy guard fired exactly right on +9202 — `RemoveStack privatebin REFUSED (busy): a backup or restore is running (single-flight held)`, +with the household's own sentence — and answered **HTTP 500**. The status mapping in +`router.go` greps the error TEXT for `not deployed` / `still running` / `not found` / `protected`, +and the busy sentence contains none of them, so it fell through to the default. + +**A 500 tells the UI something broke. This is "wait a moment".** The refusal is now a typed +`*stacks.RemoveBusyError` the handler recognises with `errors.As`, answered **409**, carrying both +the Hungarian bytes and the bundle key. Its test asserts the sentence contains none of the words the +text mapping greps for — so the type is load-bearing rather than decorative. + ## v0.262.0 — six defects two drill nights found in the update, remove and hold paths (2026-09-22, R-630/R-634/R-633/R-626/R-621/R-614) **MinAgent: 0.131.0** (unchanged). **Two** new sentences, each BORN AS A KEY in both bundles and diff --git a/controller/internal/api/router.go b/controller/internal/api/router.go index a4ada04..c774631 100644 --- a/controller/internal/api/router.go +++ b/controller/internal/api/router.go @@ -911,6 +911,15 @@ func (r *Router) removeStack(w http.ResponseWriter, req *http.Request, name stri writeJSON(w, http.StatusConflict, apiResponse{OK: false, Error: refused.Message}) return } + // R-633: "the backup side owns this app right now" is a CONFLICT, not a server error. It is + // matched by TYPE rather than by prose — the status mapping below greps the error text, and + // the busy sentence contains none of the words it looks for, so the first live run of this + // guard answered 500 while doing exactly the right thing. + var busy *stacks.RemoveBusyError + if errors.As(err, &busy) { + 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 diff --git a/controller/internal/stacks/delete.go b/controller/internal/stacks/delete.go index e4ca057..d447a26 100644 --- a/controller/internal/stacks/delete.go +++ b/controller/internal/stacks/delete.go @@ -12,7 +12,6 @@ import ( "time" "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" - "gitea.dooplex.hu/admin/felhom-controller/internal/util" ) // felhomDataDir matches backup.FelhomDataDir — duplicated to avoid circular import via StackDataProvider. @@ -468,6 +467,22 @@ func (m *Manager) verifyTornDown(name string, window time.Duration) (bool, []str return true, removed } +// RemoveBusyError is a removal refused because the backup side owns the app right now. It is a +// TYPED error, not a sentence the handler pattern-matches: the first live run of this guard answered +// **500** because the status mapping greps the error TEXT for "not deployed"/"still running" and the +// busy sentence contains neither. A 500 tells the UI something broke; this is a "wait a moment". +type RemoveBusyError struct { + Why string // the guard's own words, for logs — never shown to the household +} + +func (e *RemoveBusyError) Error() string { return MsgRemoveBusyHU } + +// Key lets the API localise it; same contract as util.MsgError. +func (e *RemoveBusyError) Key() string { return KeyRemoveBusy } + +// MsgRemoveBusyHU is the default-language bytes, matching the bundle entry for KeyRemoveBusy. +const MsgRemoveBusyHU = "Az alkalmazáson mentés vagy visszaállítás fut. Várd meg, amíg befejeződik." + // KeyRemoveBusy is the one sentence R-633 adds: a remove refused because the backup side owns the app. const KeyRemoveBusy = "err.stacks.az_alkalmazason_mentes_vagy_visszaallitas_fut" @@ -540,12 +555,12 @@ func (m *Manager) RemoveStack(name string, removeHDDData bool, backupPathsToRemo if g := m.guards(); g != nil { if busy, why := g.Busy(name); busy { m.logger.Printf("[ERROR] [stacks] RemoveStack %s REFUSED (busy): %s", name, why) - return nil, util.MsgError(KeyRemoveBusy) + return nil, &RemoveBusyError{Why: why} } } if m.IsUpdating(name) { m.logger.Printf("[ERROR] [stacks] RemoveStack %s REFUSED (busy): a guarded update is in progress", name) - return nil, util.MsgError(KeyRemoveBusy) + return nil, &RemoveBusyError{Why: "a guarded update is in progress"} } // Must be stopped (not running) diff --git a/controller/internal/stacks/r630_r634_v0262_test.go b/controller/internal/stacks/r630_r634_v0262_test.go index 621087d..0eda98b 100644 --- a/controller/internal/stacks/r630_r634_v0262_test.go +++ b/controller/internal/stacks/r630_r634_v0262_test.go @@ -1,6 +1,7 @@ package stacks import ( + "errors" "os" "path/filepath" "strings" @@ -156,3 +157,35 @@ func TestClearUpdateState(t *testing.T) { } m.ClearUpdateState("does-not-exist") // must not panic } + +// ── B — the busy refusal is a CONFLICT, by type (R-633) ────────────────────────────────────────── +// +// RED-PROOF: return `util.MsgError(KeyRemoveBusy)` instead of `&RemoveBusyError{}` and this fails — +// which is what the first live run did, answering **500** while refusing correctly. The status +// mapping greps the error TEXT for "not deployed"/"still running"; the busy sentence has neither. + +func TestRemoveBusyError_IsTypedAndCarriesBothForms(t *testing.T) { + var err error = &RemoveBusyError{Why: "a backup or restore is running (single-flight held)"} + + var busy *RemoveBusyError + if !errors.As(err, &busy) { + t.Fatal("the API must be able to recognise this by TYPE, not by matching prose") + } + if busy.Why == "" { + t.Error("the guard's own words must survive for the log") + } + if err.Error() != MsgRemoveBusyHU { + t.Errorf("the default-language bytes must be the bundle's Hungarian, got %q", err.Error()) + } + if busy.Key() != KeyRemoveBusy { + t.Errorf("the key must travel so the API can localise it, got %q", busy.Key()) + } + // The whole point: none of the words the status mapping greps for appear in this sentence, so a + // text-matching handler WOULD answer 500. This assertion is why the type exists. + for _, w := range []string{"not deployed", "still running", "not found", "protected"} { + if strings.Contains(err.Error(), w) { + t.Errorf("the sentence happens to contain %q — the text mapping would catch it and this "+ + "test would stop proving anything", w) + } + } +}