From 2e07d4c79c7c432f9f49d9cbd7a87ee1fc4ea428 Mon Sep 17 00:00:00 2001 From: Raj Nakarja Date: Fri, 21 Aug 2026 10:17:02 +0200 Subject: [PATCH] Confirm before handing a fleet away, and say what a release does fleet transfer was the one destructive verb that sent its request with no question asked: piping "n" into it still moved the fleet, its devices and its credit to another account, and the server never re-adds the old owner. It now resolves the fleet, names it and the recipient, and reads the answer first. device release asked about the fleet and never the device, so every device in a fleet produced the same question and a mistyped IMEI read exactly like the device meant. It names the device by its own name now, falling back to the IMEI when it has none. Both verbs claimed a release erases everything on the device. It clears the fleet, after which the device wipes its files and restarts its code, so the firmware, the device key and the SIM identity all survive. The table entry, the release question and the fleet delete question say that instead. Key stands alone nowhere a user reads: the table section, the empty lists, the revoke question and both key success lines say fleet key. The KEY column header stays, since the columns beside it already name the fleet and widening it would skew every row against the server's five-character suffix. Success lines name what they acted on, the way a question already does, so "Deleted %q." and "Released the device." became "Deleted fleet %q." and "Released device %q from fleet %q." fleet delete, key create, key revoke and member remove printed a line no test read, which is how the old wording drifted unnoticed. Each asserts it now, and every new assertion was checked against the deletion it exists to catch. No version change, and nothing under .github, flake.nix, .goreleaser.yaml or go.mod is touched. --- internal/device/device.go | 20 +++++--- internal/device/device_test.go | 19 +++++--- internal/fleet/fleet.go | 45 +++++++++++++++--- internal/fleet/fleet_test.go | 84 ++++++++++++++++++++++++++++------ internal/key/key.go | 10 ++-- internal/key/key_test.go | 10 ++-- internal/member/member.go | 6 +-- internal/member/member_test.go | 5 +- main.go | 10 ++-- 9 files changed, 157 insertions(+), 52 deletions(-) diff --git a/internal/device/device.go b/internal/device/device.go index e09adc9..16dfc36 100644 --- a/internal/device/device.go +++ b/internal/device/device.go @@ -86,7 +86,7 @@ func Claim(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintf(session.Out, "Claimed the device into %q.\n", fleetName) + fmt.Fprintf(session.Out, "Claimed device %s into fleet %q.\n", imei, fleetName) return nil } @@ -297,7 +297,7 @@ func Rename(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintf(session.Out, "Renamed the device to %q.\n", name) + fmt.Fprintf(session.Out, "Renamed device %s to %q.\n", imei, name) return nil } @@ -320,10 +320,18 @@ func Release(session api.Session, arguments []string) error { } fleetId := int64(0) + label := "" for _, device := range devices { - if device.Imei == imei { - fleetId = device.FleetId + if device.Imei != imei { + continue + } + + fleetId = device.FleetId + label = imei + + if device.Name != nil && *device.Name != "" { + label = *device.Name } } @@ -349,7 +357,7 @@ func Release(session api.Session, arguments []string) error { return errors.New("no such device, device list shows yours") } - fmt.Fprintf(session.Out, "Release the device from %q? It erases everything on the device, and claiming it again means pressing its pairing button in person. [y/N] ", fleetName) + fmt.Fprintf(session.Out, "Release device %q from fleet %q? It wipes the device's files and restarts its code, and claiming it again means pressing its pairing button in person. [y/N] ", label, fleetName) answer, _ := bufio.NewReader(session.In).ReadString('\n') @@ -378,7 +386,7 @@ func Release(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintln(session.Out, "Released the device.") + fmt.Fprintf(session.Out, "Released device %q from fleet %q.\n", label, fleetName) return nil } diff --git a/internal/device/device_test.go b/internal/device/device_test.go index a33bf42..3aa025c 100644 --- a/internal/device/device_test.go +++ b/internal/device/device_test.go @@ -23,7 +23,7 @@ func TestDeviceClaim(t *testing.T) { { name: "button pressed", statusCode: http.StatusNoContent, - wantOutput: "Press the pairing button on the device to finish claiming it.\nClaimed the device into \"pilot\".\n", + wantOutput: "Press the pairing button on the device to finish claiming it.\nClaimed device 354820091234567 into fleet \"pilot\".\n", }, { name: "button not pressed", @@ -117,7 +117,7 @@ func TestDeviceClaimOmitsAnAbsentName(t *testing.T) { t.Error("the request included a name although none was given") } - if out.String() != "Press the pairing button on the device to finish claiming it.\nClaimed the device into \"pilot\".\n" { + if out.String() != "Press the pairing button on the device to finish claiming it.\nClaimed device 354820091234567 into fleet \"pilot\".\n" { t.Errorf("output = %q", out.String()) } } @@ -264,7 +264,7 @@ func TestDeviceRename(t *testing.T) { wantOutput string wantError string }{ - {name: "renamed", wantOutput: "Renamed the device to \"pilot\".\n"}, + {name: "renamed", wantOutput: "Renamed device 354820091234567 to \"pilot\".\n"}, {name: "server refusal", refusal: "no such device", wantError: "no such device"}, } @@ -341,14 +341,16 @@ func TestDeviceRelease(t *testing.T) { tests := []struct { name string answer string + devices string fleets string refusal string wantReleased bool wantOutput string wantError string }{ - {name: "confirmed", answer: "yes\n", wantReleased: true, wantOutput: "Release the device from \"pilot\"? It erases everything on the device, and claiming it again means pressing its pairing button in person. [y/N] Released the device.\n"}, - {name: "declined", answer: "n\n", wantOutput: "Release the device from \"pilot\"? It erases everything on the device, and claiming it again means pressing its pairing button in person. [y/N] Nothing released.\n"}, + {name: "confirmed", answer: "yes\n", wantReleased: true, wantOutput: "Release device \"354820091234567\" from fleet \"pilot\"? It wipes the device's files and restarts its code, and claiming it again means pressing its pairing button in person. [y/N] Released device \"354820091234567\" from fleet \"pilot\".\n"}, + {name: "declined", answer: "n\n", wantOutput: "Release device \"354820091234567\" from fleet \"pilot\"? It wipes the device's files and restarts its code, and claiming it again means pressing its pairing button in person. [y/N] Nothing released.\n"}, + {name: "a named device is named back, not its IMEI", answer: "n\n", devices: `[{"imei":"354820091234567","name":"rooftop","fleet_id":3,"last_seen_at":null}]`, wantOutput: "Release device \"rooftop\" from fleet \"pilot\"? It wipes the device's files and restarts its code, and claiming it again means pressing its pairing button in person. [y/N] Nothing released.\n"}, {name: "server refuses", answer: "y\n", refusal: "no such device", wantReleased: true, wantError: "no such device"}, {name: "device belongs to an inaccessible fleet", fleets: `[]`, wantError: "no such device, device list shows yours"}, } @@ -357,14 +359,19 @@ func TestDeviceRelease(t *testing.T) { t.Run(test.name, func(t *testing.T) { releasedPath := "" fleets := test.fleets + devices := test.devices if fleets == "" { fleets = `[{"id":3,"name":"pilot","owner":true}]` } + if devices == "" { + devices = `[{"imei":"354820091234567","name":null,"fleet_id":3,"last_seen_at":null}]` + } + mux := http.NewServeMux() mux.HandleFunc("GET /devices", func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, `[{"imei":"354820091234567","name":null,"fleet_id":3,"last_seen_at":null}]`) + fmt.Fprint(w, devices) }) mux.HandleFunc("GET /fleets", func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, fleets) diff --git a/internal/fleet/fleet.go b/internal/fleet/fleet.go index 9a9dcdb..ea01162 100644 --- a/internal/fleet/fleet.go +++ b/internal/fleet/fleet.go @@ -149,7 +149,7 @@ func Rename(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintf(session.Out, "Renamed the fleet to %q.\n", name) + fmt.Fprintf(session.Out, "Renamed fleet %d to %q.\n", fleetId, name) return nil } @@ -167,6 +167,37 @@ func Transfer(session api.Session, arguments []string) error { email := arguments[1] + fleets, err := api.FetchFleets(session) + + if err != nil { + return err + } + + name := "" + found := false + + for _, fleet := range fleets { + if fleet.Id == fleetId { + name = fleet.Name + found = true + } + } + + if !found { + return errors.New("no such fleet") + } + + fmt.Fprintf(session.Out, "Hand fleet %q to %s? They become the owner, and you lose access to the fleet, its devices and its credit. [y/N] ", name, email) + + answer, _ := bufio.NewReader(session.In).ReadString('\n') + + answer = strings.ToLower(strings.TrimSpace(answer)) + + if answer != "y" && answer != "yes" { + fmt.Fprintln(session.Out, "Nothing transferred.") + return nil + } + body, err := json.Marshal(map[string]string{"email": email}) if err != nil { @@ -194,7 +225,7 @@ func Transfer(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintf(session.Out, "Transferred the fleet to %s.\n", email) + fmt.Fprintf(session.Out, "Transferred fleet %q to %s.\n", name, email) return nil } @@ -256,14 +287,14 @@ func Delete(session api.Session, arguments []string) error { } } - consequence := "It erases them all, and claiming one again means pressing its pairing button in person." + consequence := "It wipes their files and restarts their code, and claiming one again means pressing its pairing button in person." if forfeitUnknown { - fmt.Fprintf(session.Out, "Delete %q, release its devices, and forfeit its remaining credit? %s [y/N] ", name, consequence) + fmt.Fprintf(session.Out, "Delete fleet %q, release its devices, and forfeit its remaining credit? %s [y/N] ", name, consequence) } else if forfeited == "" { - fmt.Fprintf(session.Out, "Delete %q and release its devices? %s [y/N] ", name, consequence) + fmt.Fprintf(session.Out, "Delete fleet %q and release its devices? %s [y/N] ", name, consequence) } else { - fmt.Fprintf(session.Out, "Delete %q, release its devices, and forfeit its remaining %s of credit? %s [y/N] ", name, forfeited, consequence) + fmt.Fprintf(session.Out, "Delete fleet %q, release its devices, and forfeit its remaining %s of credit? %s [y/N] ", name, forfeited, consequence) } answer, _ := bufio.NewReader(session.In).ReadString('\n') @@ -294,7 +325,7 @@ func Delete(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintf(session.Out, "Deleted %q.\n", name) + fmt.Fprintf(session.Out, "Deleted fleet %q.\n", name) return nil } diff --git a/internal/fleet/fleet_test.go b/internal/fleet/fleet_test.go index 52a5c5d..e6b8327 100644 --- a/internal/fleet/fleet_test.go +++ b/internal/fleet/fleet_test.go @@ -171,7 +171,7 @@ func TestFleetRename(t *testing.T) { wantOutput string wantError string }{ - {name: "renamed", wantOutput: "Renamed the fleet to \"pilot\".\n"}, + {name: "renamed", wantOutput: "Renamed fleet 9 to \"pilot\".\n"}, {name: "server refusal", refusal: "no such fleet", wantError: "no such fleet"}, } @@ -247,21 +247,59 @@ func TestFleetRenameArguments(t *testing.T) { func TestFleetTransfer(t *testing.T) { tests := []struct { - name string - refusal string - wantOutput string - wantError string + name string + answer string + fleets string + refusal string + wantTransferred bool + wantOutput string + wantError string }{ - {name: "transferred", wantOutput: "Transferred the fleet to successor@example.com.\n"}, - {name: "server refusal", refusal: "the new owner has no account", wantError: "the new owner has no account"}, + { + name: "confirmed with y", + answer: "y\n", + wantTransferred: true, + wantOutput: "Hand fleet \"pilot\" to successor@example.com? They become the owner, and you lose access to the fleet, its devices and its credit. [y/N] Transferred fleet \"pilot\" to successor@example.com.\n", + }, + {name: "confirmed with yes", answer: "YES\n", wantTransferred: true}, + { + name: "declined with n", + answer: "n\n", + wantOutput: "Hand fleet \"pilot\" to successor@example.com? They become the owner, and you lose access to the fleet, its devices and its credit. [y/N] Nothing transferred.\n", + }, + {name: "declined by default", answer: "\n"}, + {name: "closed input", answer: ""}, + { + name: "the server refuses after the confirmation", + answer: "y\n", + refusal: "no one with that email address has logged in yet", + wantTransferred: true, + wantError: "no one with that email address has logged in yet", + }, + { + name: "a fleet that is not yours", + answer: "y\n", + fleets: `[]`, + wantError: "no such fleet", + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { transferredPath := "" transferredTo := "" + fleets := test.fleets + + if fleets == "" { + fleets = `[{"id":3,"name":"pilot","owner":true}]` + } + mux := http.NewServeMux() + mux.HandleFunc("GET /fleets", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, fleets) + }) + mux.HandleFunc("POST /fleets/{id}/owner", func(w http.ResponseWriter, r *http.Request) { body := struct { Email string `json:"email"` @@ -281,23 +319,35 @@ func TestFleetTransfer(t *testing.T) { session, out := apitest.LoggedInSession(t, mux) + session.In = strings.NewReader(test.answer) + err := Transfer(session, []string{"3", "successor@example.com"}) + printed := out.String() + if test.wantError != "" { if err == nil || err.Error() != test.wantError { t.Fatalf("error = %v, want %q", err, test.wantError) } + + if strings.Contains(printed, "Transferred") { + t.Errorf("the output %q says the fleet was handed over although it was not", printed) + } } else if err != nil { t.Fatal(err) } - if transferredPath != "/fleets/3/owner" || transferredTo != "successor@example.com" { + if test.wantOutput != "" && printed != test.wantOutput { + t.Errorf("output = %q, want %q", printed, test.wantOutput) + } + + if test.wantTransferred && (transferredPath != "/fleets/3/owner" || transferredTo != "successor@example.com") { t.Errorf("the server saw %q handed to %q, want %q handed to %q", transferredPath, transferredTo, "/fleets/3/owner", "successor@example.com") } - if out.String() != test.wantOutput { - t.Errorf("output = %q, want %q", out.String(), test.wantOutput) + if !test.wantTransferred && transferredPath != "" { + t.Errorf("the server saw %q handed over although the confirmation was declined", transferredPath) } }) } @@ -397,6 +447,10 @@ func TestFleetDelete(t *testing.T) { t.Errorf("the server saw %q deleted, want %q", deletedPath, "/fleets/3") } + if test.wantDeleted && test.wantError == "" && !strings.Contains(printed, "Deleted fleet \"pilot\".") { + t.Errorf("the output %q does not name the fleet it deleted", printed) + } + if !test.wantDeleted && deletedPath != "" { t.Errorf("the server saw %q deleted although the confirmation was declined", deletedPath) } @@ -414,18 +468,18 @@ func TestFleetDeletePromptStatesForfeitedCredit(t *testing.T) { { name: "remaining credit is stated", balance: `[{"fleet":3,"balance":"12.340000","currency":"eur"}]`, - wantOutput: "Delete \"pilot\", release its devices, and forfeit its remaining €12.34 of credit? It erases them all, and claiming one again means pressing its pairing button in person. [y/N] Nothing deleted.\n", + wantOutput: "Delete fleet \"pilot\", release its devices, and forfeit its remaining €12.34 of credit? It wipes their files and restarts their code, and claiming one again means pressing its pairing button in person. [y/N] Nothing deleted.\n", }, { name: "an empty balance stays quiet", balance: `[{"fleet":3,"balance":"0","currency":"eur"}]`, - wantOutput: "Delete \"pilot\" and release its devices? It erases them all, and claiming one again means pressing its pairing button in person. [y/N] Nothing deleted.\n", + wantOutput: "Delete fleet \"pilot\" and release its devices? It wipes their files and restarts their code, and claiming one again means pressing its pairing button in person. [y/N] Nothing deleted.\n", wantAbsent: "forfeit", }, { name: "an unparseable balance warns without an amount", balance: `[{"fleet":3,"balance":"15,00","currency":"eur"}]`, - wantOutput: "Delete \"pilot\", release its devices, and forfeit its remaining credit? It erases them all, and claiming one again means pressing its pairing button in person. [y/N] Nothing deleted.\n", + wantOutput: "Delete fleet \"pilot\", release its devices, and forfeit its remaining credit? It wipes their files and restarts their code, and claiming one again means pressing its pairing button in person. [y/N] Nothing deleted.\n", wantAbsent: "€", }, } @@ -458,8 +512,8 @@ func TestFleetDeletePromptStatesForfeitedCredit(t *testing.T) { t.Errorf("output = %q, want %q", printed, test.wantOutput) } - if !strings.Contains(printed, "It erases them all, and claiming one again means pressing its pairing button in person.") { - t.Errorf("the prompt %q does not say the devices are erased", printed) + if !strings.Contains(printed, "It wipes their files and restarts their code, and claiming one again means pressing its pairing button in person.") { + t.Errorf("the prompt %q does not say what releasing the devices does to them", printed) } if test.wantAbsent != "" && strings.Contains(printed, test.wantAbsent) { diff --git a/internal/key/key.go b/internal/key/key.go index 273fc86..8f44fc2 100644 --- a/internal/key/key.go +++ b/internal/key/key.go @@ -62,7 +62,7 @@ func Create(session api.Session, arguments []string) error { return err } - fmt.Fprintf(session.Out, "Created key %d.\n\n %s\n\nAnyone holding it can send data to the fleet, and you will not see it again.\n", created.Id, created.Key) + fmt.Fprintf(session.Out, "Created fleet key %d.\n\n %s\n\nAnyone holding it can send data to the fleet, and you will not see it again.\n", created.Id, created.Key) return nil } @@ -124,9 +124,9 @@ func List(session api.Session, arguments []string) error { if len(keys) == 0 { if chosenFleetId == 0 { - fmt.Fprintln(session.Out, "No keys yet. Create one with key create.") + fmt.Fprintln(session.Out, "No fleet keys yet. Create one with key create.") } else { - fmt.Fprintln(session.Out, "No keys in that fleet.") + fmt.Fprintln(session.Out, "No fleet keys on that fleet yet.") } return nil @@ -184,7 +184,7 @@ func Revoke(session api.Session, arguments []string) error { return errors.New("no such key") } - fmt.Fprintf(session.Out, "Revoke %q? Anything still using it stops reaching the fleet. [y/N] ", label) + fmt.Fprintf(session.Out, "Revoke fleet key %q? Anything still using it stops reaching the fleet. [y/N] ", label) answer, _ := bufio.NewReader(session.In).ReadString('\n') @@ -214,7 +214,7 @@ func Revoke(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintf(session.Out, "Revoked key %d.\n", keyId) + fmt.Fprintf(session.Out, "Revoked fleet key %q.\n", label) return nil } diff --git a/internal/key/key_test.go b/internal/key/key_test.go index 14ca451..70d082a 100644 --- a/internal/key/key_test.go +++ b/internal/key/key_test.go @@ -115,6 +115,10 @@ func TestKeyCreate(t *testing.T) { if !strings.Contains(printed, "you will not see it again") { t.Errorf("the output %q does not warn that the key cannot be shown again", printed) } + + if !strings.Contains(printed, "Created fleet key 1.") { + t.Errorf("the output %q does not name the fleet key it created", printed) + } }) } } @@ -149,13 +153,13 @@ func TestKeyList(t *testing.T) { { name: "no keys", arguments: []string{}, - wantShown: []string{"No keys yet. Create one with key create."}, + wantShown: []string{"No fleet keys yet. Create one with key create."}, keys: `[]`, }, { name: "a fleet without keys", arguments: []string{"5"}, - wantShown: []string{"No keys in that fleet."}, + wantShown: []string{"No fleet keys on that fleet yet."}, wantHidden: []string{"ID FLEET"}, }, { @@ -253,7 +257,7 @@ func TestKeyRevoke(t *testing.T) { arguments: []string{"3"}, answer: "y\n", wantRevoked: "/keys/3", - wantShown: "production", + wantShown: "Revoked fleet key \"production\".", }, { name: "declined by default", diff --git a/internal/member/member.go b/internal/member/member.go index deff734..4993e84 100644 --- a/internal/member/member.go +++ b/internal/member/member.go @@ -54,7 +54,7 @@ func Add(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintf(session.Out, "Gave %s access.\n", email) + fmt.Fprintf(session.Out, "Gave %s access to fleet %d.\n", email, fleetId) return nil } @@ -156,7 +156,7 @@ func Remove(session api.Session, arguments []string) error { return errors.New("no such fleet") } - fmt.Fprintf(session.Out, "Take away %s's access to %q? [y/N] ", email, name) + fmt.Fprintf(session.Out, "Take away %s's access to fleet %q? [y/N] ", email, name) answer, _ := bufio.NewReader(session.In).ReadString('\n') @@ -186,7 +186,7 @@ func Remove(session api.Session, arguments []string) error { return api.ServerError(response) } - fmt.Fprintf(session.Out, "Removed access for %s.\n", email) + fmt.Fprintf(session.Out, "Removed %s's access to fleet %q.\n", email, name) return nil } diff --git a/internal/member/member_test.go b/internal/member/member_test.go index 76dc2c8..f8863ac 100644 --- a/internal/member/member_test.go +++ b/internal/member/member_test.go @@ -18,7 +18,7 @@ func TestMemberAdd(t *testing.T) { wantOutput string wantError string }{ - {name: "added", wantOutput: "Gave member@example.com access.\n"}, + {name: "added", wantOutput: "Gave member@example.com access to fleet 3.\n"}, {name: "server refusal", refusal: "no such account", wantError: "no such account"}, } @@ -217,7 +217,8 @@ func TestMemberRemove(t *testing.T) { }{ {name: "a plain address", email: "member@example.com", answer: "y\n", wantRemoved: true}, {name: "an address with a hash", email: "a#b@example.com", answer: "yes\n", wantRemoved: true}, - {name: "the prompt names the fleet", email: "member@example.com", answer: "y\n", wantRemoved: true, wantShown: `access to "pilot"`}, + {name: "the question names the fleet", email: "member@example.com", answer: "n\n", wantShown: `Take away member@example.com's access to fleet "pilot"?`}, + {name: "the success line names the fleet", email: "member@example.com", answer: "y\n", wantRemoved: true, wantShown: `Removed member@example.com's access to fleet "pilot".`}, {name: "declined by default", email: "member@example.com", answer: "\n", wantShown: "Nothing removed"}, {name: "declined with n", email: "member@example.com", answer: "n\n", wantShown: "Nothing removed"}, {name: "closed input", email: "member@example.com", wantShown: "Nothing removed"}, diff --git a/main.go b/main.go index a69464a..372987d 100644 --- a/main.go +++ b/main.go @@ -39,7 +39,7 @@ var sections = []dispatch.Section{ {Name: "device claim", Arguments: " [name]", Summary: "Claim a device into a fleet, then press its pairing button", Run: device.Claim}, {Name: "device list", Arguments: "[fleet_id] [--json]", Summary: "List devices, their state, and when they were last seen", Run: device.List}, {Name: "device rename", Arguments: " ", Summary: "Rename a device", Run: device.Rename}, - {Name: "device release", Arguments: "", Summary: "Release a device from its fleet and erase everything on it", Run: device.Release}, + {Name: "device release", Arguments: "", Summary: "Release a device from its fleet, wiping its files and restarting its code", Run: device.Release}, {Name: "device start", Arguments: "", Summary: "Start the code on a device"}, {Name: "device stop", Arguments: "", Summary: "Stop the code on a device"}, {Name: "device restart", Arguments: "", Summary: "Restart the code on a device"}, @@ -68,11 +68,11 @@ var sections = []dispatch.Section{ }, }, { - Title: "Keys", + Title: "Fleet keys", Commands: []dispatch.Command{ - {Name: "key create", Arguments: "