provider: a logins.json magpie can't read keeps the accounts read before, and one that doesn't parse is copied aside before it is written over, not read as no accounts

readLogins dropped filememo.Read's error and json.Unmarshal's, so a file
half there (a write by another process, a disk hiccup) read as an empty
list, and the next add, switch or sign-out wrote that list back: every
account gone. d5619d5a only retried for 15ms. Now a failed read returns
the accounts last read or written (a missing file is still none), and
writeLogins copies a file that doesn't parse to logins.json.bad-<time>
before replacing it, so what was in it can be got back.

TestUnreadLoginsNotWrittenOver is new: it truncates logins.json and
checks the accounts are still read and survive the next add, and that a
truncated file with nothing read before is kept aside. It fails on
origin/main ("read as []") and passes with the change. go vet, go build
(darwin, GOOS=linux nogui, GOOS=windows) and go test -tags nogui ./...
under a temp HOME pass; the provider package passes -race.

internal/provider/logins.go

@@ -13,6 +13,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/fs"
+	"log"
 	"os"
 	"path/filepath"
 	"slices"
@@ -112,23 +114,60 @@ var loginAgents = []string{"claude", "codex"}
 
 func loginsPath() string { return filepath.Join(filepath.Dir(Path()), "logins.json") }
 
+// lastLogins is the accounts last read from logins.json: a read that fails
+// (a file half there, one magpie can't open for a moment) is them, not no
+// accounts, which the next change of an account would write back over
+// every account.
+var (
+	lastLoginsMu sync.Mutex
+	lastLogins   []savedLogin
+)
+
 func readLogins() []savedLogin {
 	// parsed once until the file changes: a state of the page asks for it
 	// dozens of times (every agent's drift and models), and with the
 	// accounts' credentials in it the file is large — a Save of a profile
 	// waited seconds on it
-	ls, _ := filememo.Read("logins", loginsPath(), func(b []byte) ([]savedLogin, error) {
+	ls, err := filememo.Read("logins", loginsPath(), func(b []byte) ([]savedLogin, error) {
 		var out []savedLogin
-		_ = json.Unmarshal(b, &out)
+		if err := json.Unmarshal(b, &out); err != nil {
+			return nil, err
+		}
 		// DimAgent's accounts: magpie no longer signs in to it (DimAgent
 		// doesn't allow its subscription used outside its client), so one
 		// signed in before is left out, and gone from the file at its next write
 		out = slices.DeleteFunc(out, func(l savedLogin) bool { return l.Agent == "dimagent" })
 		return nameAlike(dedupeLogins(out)), nil
 	})
+	lastLoginsMu.Lock()
+	defer lastLoginsMu.Unlock()
+	switch {
+	case err == nil:
+		lastLogins = ls
+	case errors.Is(err, fs.ErrNotExist):
+		lastLogins = nil
+	default:
+		log.Printf("logins.json: %v; the accounts read before are kept", err)
+		ls = lastLogins
+	}
 	return slices.Clone(ls) // callers change theirs
 }
 
+// keepUnreadLogins copies a logins.json that doesn't parse aside before it
+// is written over, so the accounts in it can still be got back.
+func keepUnreadLogins(path string) {
+	b, err := os.ReadFile(path)
+	if err != nil || json.Valid(b) {
+		return
+	}
+	bad := path + ".bad-" + time.Now().Format("20060102-150405")
+	if err := os.WriteFile(bad, b, 0o600); err != nil {
+		log.Printf("logins.json doesn't parse and couldn't be kept: %v", err)
+		return
+	}
+	log.Printf("logins.json didn't parse; it is kept as %s", filepath.Base(bad))
+}
+
 func writeLogins(ls []savedLogin) error {
 	sort.SliceStable(ls, func(i, j int) bool {
 		if ls[i].Agent != ls[j].Agent {
@@ -150,7 +189,14 @@ func writeLogins(ls []savedLogin) error {
 		return err
 	}
 	defer Changed() // an account added, switched or gone: All builds anew
-	return writePrivate(loginsPath(), append(b, '\n'))
+	keepUnreadLogins(loginsPath())
+	if err := writePrivate(loginsPath(), append(b, '\n')); err != nil {
+		return err
+	}
+	lastLoginsMu.Lock()
+	lastLogins = slices.Clone(ls)
+	lastLoginsMu.Unlock()
+	return nil
 }
 
 // writePrivate replaces a file readable by the user alone, atomically, so

internal/provider/logins_unread_test.go

@@ -0,0 +1,64 @@
+package provider
+
+import (
+	"os"
+	"path/filepath"
+	"slices"
+	"strings"
+	"testing"
+)
+
+// A logins.json that doesn't parse (half written, or written by something
+// else) was read as no accounts, and the next account added wrote the
+// file anew with that one alone: every saved sign-in gone.
+func TestUnreadLoginsNotWrittenOver(t *testing.T) {
+	googleSandbox(t, &fakeGoogle{})
+	for _, u := range []string{"a@x.com", "b@x.com"} {
+		if err := addGoogleLogin("antigravity", u, "", googleAuth{RefreshToken: "1//" + u, Project: "p"}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	p := loginsPath()
+	good, err := os.ReadFile(p)
+	if err != nil {
+		t.Fatal(err)
+	}
+	users := func() []string {
+		var us []string
+		for _, l := range readLogins() {
+			us = append(us, l.User)
+		}
+		slices.Sort(us)
+		return us
+	}
+
+	// a read that fails: the accounts read before are kept
+	os.WriteFile(p, good[:len(good)/2], 0o600)
+	if us := users(); strings.Join(us, ",") != "a@x.com,b@x.com" {
+		t.Fatalf("a logins.json half there read as %v", us)
+	}
+	if err := addGoogleLogin("antigravity", "c@x.com", "", googleAuth{RefreshToken: "1//c", Project: "p"}); err != nil {
+		t.Fatal(err)
+	}
+	if us := users(); strings.Join(us, ",") != "a@x.com,b@x.com,c@x.com" {
+		t.Fatalf("after an account added: %v", us)
+	}
+
+	// nothing read before (magpie just started): the file is kept aside
+	// before it is written over
+	half := good[:len(good)/2]
+	os.WriteFile(p, half, 0o600)
+	lastLoginsMu.Lock()
+	lastLogins = nil
+	lastLoginsMu.Unlock()
+	if err := addGoogleLogin("antigravity", "d@x.com", "", googleAuth{RefreshToken: "1//d", Project: "p"}); err != nil {
+		t.Fatal(err)
+	}
+	kept, _ := filepath.Glob(p + ".bad-*")
+	if len(kept) != 1 {
+		t.Fatalf("logins.json that didn't parse not kept aside: %v", kept)
+	}
+	if b, _ := os.ReadFile(kept[0]); string(b) != string(half) {
+		t.Fatalf("kept %q", b)
+	}
+}