From ac3a557566b198dea385295b1a5c55306cec7e4c Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Wed, 23 Sep 2026 07:49:07 +0800 Subject: [PATCH] fix(cli): Sync picker hides system servers; keep the two 409 refusals apart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects from the live Sync drill: - The picker listed the system servers (login/lobby), which the backup API can never accept (reserved names, no servers row): the pick died in name validation with a raw "server name is reserved" error. backupPickable now filters them out; the halt picker keeps them on purpose (break-glass retains full power over system servers). - backupErrorFromResponse mapped every 409 to the stopped gate, so the new world-volume refusal would have displayed the wrong reason. The 409 arm now keys on the body's error code; a code-less body still reads as the stopped gate. Live (auditfix38): the picker shows only user servers; a world-less pick shows the API's own "no world volume yet — start it once" text; the not_stopped text is unchanged. --- cmd/felis/backupnow.go | 42 +++++++++++++++++++++++++++++-------- cmd/felis/backupnow_test.go | 35 ++++++++++++++++++++++++++++--- cmd/felis/tui_backupnow.go | 2 +- 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/cmd/felis/backupnow.go b/cmd/felis/backupnow.go index a7e462f..fb558e2 100644 --- a/cmd/felis/backupnow.go +++ b/cmd/felis/backupnow.go @@ -86,23 +86,31 @@ func requestBackup(ctx context.Context, hc *http.Client, baseURL, token, name, o // backupErrorFromResponse turns a non-202 into a human message. The well-known codes get // an operator-facing explanation; anything else falls back to the API's -// {"error":{message}} body, then the bare status code. +// {"error":{code,message}} body, then the bare status code. func backupErrorFromResponse(resp *http.Response) error { - switch resp.StatusCode { - case http.StatusConflict: // not_stopped - return fmt.Errorf("the server must be stopped before its world can be backed up — halt it first") - case http.StatusServiceUnavailable: // backup_unavailable - return fmt.Errorf("the backup subsystem is not configured on felis-api (FELIS_IMAGE / FELIS_BACKUP_PVC unset)") - case http.StatusNotFound: - return fmt.Errorf("no such server") - } var e struct { Error struct { + Code string `json:"code"` Message string `json:"message"` } `json:"error"` } raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<16)) _ = json.Unmarshal(raw, &e) + + switch resp.StatusCode { + case http.StatusConflict: + // Two refusals share 409: the stopped gate and the missing-world-volume + // gate. The body's code distinguishes them; a code-less body reads as the + // stopped gate (the only 409 before the volume gate existed), and any other + // coded 409 falls through to the API's own operator text. + if e.Error.Code == "" || e.Error.Code == "not_stopped" { + return fmt.Errorf("the server must be stopped before its world can be backed up — halt it first") + } + case http.StatusServiceUnavailable: // backup_unavailable + return fmt.Errorf("the backup subsystem is not configured on felis-api (FELIS_IMAGE / FELIS_BACKUP_PVC unset)") + case http.StatusNotFound: + return fmt.Errorf("no such server") + } if e.Error.Message != "" { return fmt.Errorf("felis-api: %s", e.Error.Message) } @@ -120,3 +128,19 @@ func performBackupNow(ctx context.Context, cl client.Client, controlNamespace, n hc := &http.Client{Timeout: 10 * time.Second} return requestBackup(ctx, hc, baseURL, token, name, osUser) } + +// backupPickable narrows the backup picker to servers the backup API can accept. +// System servers (login/lobby) are excluded: they have no row in the servers +// table and carry reserved names, so every attempt dies in name validation — +// offering them would be a dead pick. The halt picker keeps them on purpose +// (break-glass retains full power over system servers); only the API-backed +// backup op cannot reach them. +func backupPickable(servers []haltableServer) []haltableServer { + out := make([]haltableServer, 0, len(servers)) + for _, s := range servers { + if !s.system { + out = append(out, s) + } + } + return out +} diff --git a/cmd/felis/backupnow_test.go b/cmd/felis/backupnow_test.go index b7de2b5..1594c2e 100644 --- a/cmd/felis/backupnow_test.go +++ b/cmd/felis/backupnow_test.go @@ -114,16 +114,23 @@ func TestRequestBackup(t *testing.T) { cases := []struct { name string code int + body string // optional JSON error body expect string }{ - {"409 not_stopped", http.StatusConflict, "must be stopped"}, - {"503 backup_unavailable", http.StatusServiceUnavailable, "not configured"}, - {"404 not found", http.StatusNotFound, "no such server"}, + {"409 not_stopped", http.StatusConflict, "", "must be stopped"}, + {"409 no_world_volume surfaces the API's own text", http.StatusConflict, + `{"error":{"code":"no_world_volume","message":"this server has no world volume yet — start it once to create it, then retry"}}`, + "no world volume yet"}, + {"503 backup_unavailable", http.StatusServiceUnavailable, "", "not configured"}, + {"404 not found", http.StatusNotFound, "", "no such server"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(tc.code) + if tc.body != "" { + _, _ = io.WriteString(w, tc.body) + } })) defer srv.Close() _, err := requestBackup(context.Background(), hc, srv.URL, "tok", "survival", "alice") @@ -142,3 +149,25 @@ func TestRequestBackup(t *testing.T) { } }) } + +// The Sync picker must not offer system servers: the backup API validates names +// and resolves a servers-table row, so a lobby/login pick can only die in +// validation — a dead choice in an emergency console. +func TestBackupPickable(t *testing.T) { + got := backupPickable([]haltableServer{ + {name: "lobby", phase: "Running", system: true}, + {name: "login", phase: "Running", system: true}, + {name: "test-one", phase: "Stopped"}, + }) + if len(got) != 1 || got[0].name != "test-one" || got[0].system { + t.Fatalf("backupPickable = %+v, want only the user server", got) + } + + // Survivors keep their input order (the picker's cursor math depends on it). + got = backupPickable([]haltableServer{ + {name: "alpha"}, {name: "login", system: true}, {name: "beta"}, + }) + if len(got) != 2 || got[0].name != "alpha" || got[1].name != "beta" { + t.Fatalf("backupPickable order = %+v, want [alpha beta]", got) + } +} diff --git a/cmd/felis/tui_backupnow.go b/cmd/felis/tui_backupnow.go index 0906d74..aeac8d1 100644 --- a/cmd/felis/tui_backupnow.go +++ b/cmd/felis/tui_backupnow.go @@ -86,7 +86,7 @@ func (m *backupModel) loadCmd() tea.Cmd { if err != nil { return backupListMsg{err: fmt.Errorf("list servers: %w", err)} } - return backupListMsg{cl: cl, servers: servers} + return backupListMsg{cl: cl, servers: backupPickable(servers)} } }