provider: keep saved credentials when reading or backing them up fails (#1034)

Follow up #965: keepUnreadLogins still ignored read and backup failures,
so a new sign-in could replace credentials that had not been recovered.
Return those errors and stop writeLogins before replacing the original.
A missing logins.json can still be created, and successful backups keep
the existing behavior. Document the save failure contract.

TestUnreadLoginsWriteErrorsKeepCredentials fails on the latest base for
both faults with "original credentials overwritten" and passes with the
fix. The unread-logins tests also pass under -race -count=20.

On 588e51ea, the full nogui suite, go vet, and macOS/Linux(nogui)/Windows
builds pass under a temporary HOME with pinned Go caches. The full suite
uses a temporary, checksum-verified Bun for the CLI's plugin test.

docs/subsystems/providers-accounts.md

@@ -44,7 +44,7 @@ keys and accounts per request is in [Gateway routing and fallback](gateway-routi
 - Each refresh token has exactly one holder. Vendors rotate tokens on refresh, so two copies of one token would sign each other out. `savedTokenMu` stops two requests refreshing one saved account at once.
 - A Claude subscription account is used only for Claude Code, through the genuine Claude Code binary (see [Claude subscription bridge](claude-subscription-bridge.md)).
 - An unreadable `providers.json` is an error, never an empty list. Saving over an empty list would lose every provider.
-- When `logins.json` cannot be decoded as saved accounts, reads keep the accounts read before. Before saving a change, `keepUnreadLogins` copies the original file aside, including valid JSON with invalid account field types, so its credentials can be recovered.
+- When `logins.json` cannot be decoded as saved accounts, reads keep the accounts read before. Before saving a change, `keepUnreadLogins` copies the original file aside, including valid JSON with invalid account field types, so its credentials can be recovered. If the existing file cannot be read or the backup fails, saving returns the error and leaves the original file unchanged. A missing file can be created normally.
 - A switched-off provider (`Off`, #163) stays saved. The gateway says it is off instead of saying the model is unknown.
 - A provider removed or switched off takes its models out of the catalog. [Agent wiring](agent-wiring.md)'s `Reseat` moves agents off them.
 - A gateway key held to some accounts (#905) is held to those of the providers its list names: a provider it names no account of, the key uses as it always did. The list keeps an account by its stable id — `logins.json`'s `id`, set once written and kept through renames; the agents that keep no logins (and plugins) are known by a hash of the account's name, so an account renamed there drops out of a key's list, holding it closer, never wider. Storing an id for every agent is a larger change, not this one's.

internal/provider/logins.go

@@ -186,23 +186,28 @@ func readLogins() []savedLogin {
 }
 
 // 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) {
+// is written over, so the accounts in it can still be got back. A failure
+// to read or keep an existing file stops the write.
+func keepUnreadLogins(path string) error {
 	b, err := os.ReadFile(path)
+	if errors.Is(err, fs.ErrNotExist) {
+		return nil
+	}
 	if err != nil {
-		return
+		return err
 	}
 	// Match readLogins: valid JSON can still have unreadable field types.
 	var ls []savedLogin
 	if json.Unmarshal(b, &ls) == nil {
-		return
+		return nil
 	}
 	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
+		return err
 	}
 	log.Printf("logins.json didn't parse; it is kept as %s", filepath.Base(bad))
+	return nil
 }
 
 func writeLogins(ls []savedLogin) error {
@@ -231,7 +236,9 @@ func writeLogins(ls []savedLogin) error {
 		return err
 	}
 	defer Changed() // an account added, switched or gone: All builds anew
-	keepUnreadLogins(loginsPath())
+	if err := keepUnreadLogins(loginsPath()); err != nil {
+		return err
+	}
 	if err := writePrivate(loginsPath(), append(b, '\n')); err != nil {
 		return err
 	}

internal/provider/logins_unread_test.go

@@ -2,13 +2,75 @@ package provider
 
 import (
 	"encoding/json"
+	"errors"
 	"os"
 	"path/filepath"
 	"slices"
 	"strings"
 	"testing"
+	"testing/synctest"
+	"time"
 )
 
+// A read or backup that fails must stop a new sign-in from replacing the
+// original credentials, even when no accounts have been read before.
+func TestUnreadLoginsWriteErrorsKeepCredentials(t *testing.T) {
+	for _, fault := range []string{"read", "backup"} {
+		t.Run(fault, func(t *testing.T) {
+			azureHome(t)
+			// Freeze the backup's timestamp so an existing directory blocks
+			// exactly its filename, without depending on the wall clock.
+			synctest.Test(t, func(t *testing.T) {
+				p := loginsPath()
+				original := `[{"agent":"gemini","user":"old@example.com","auth":{"refresh_token":"old-refresh-token"}}]`
+				if fault == "backup" {
+					original = strings.TrimSuffix(original, "]") // a file half written
+				}
+				if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil {
+					t.Fatal(err)
+				}
+				if err := os.WriteFile(p, []byte(original), 0o600); err != nil {
+					t.Fatal(err)
+				}
+				lastLoginsMu.Lock()
+				lastLogins = nil // a cold start: no cached credentials to recover
+				lastLoginsMu.Unlock()
+				blocked := p
+				if fault == "read" {
+					if err := os.Chmod(p, 0); err != nil {
+						t.Fatal(err)
+					}
+					t.Cleanup(func() { _ = os.Chmod(p, 0o600) })
+					if _, err := os.ReadFile(p); err == nil {
+						t.Skip("the filesystem does not enforce the unreadable mode")
+					} else if !errors.Is(err, os.ErrPermission) {
+						t.Fatal(err)
+					}
+				} else {
+					blocked = p + ".bad-" + time.Now().Format("20060102-150405")
+					if err := os.Mkdir(blocked, 0o700); err != nil {
+						t.Fatal(err)
+					}
+				}
+				err := addGoogleLogin("gemini", "new@example.com", "", googleAuth{RefreshToken: "new-refresh-token", Project: "p"})
+				if chmodErr := os.Chmod(p, 0o600); chmodErr != nil {
+					t.Fatal(chmodErr)
+				}
+				b, readErr := os.ReadFile(p)
+				if readErr != nil {
+					t.Fatal(readErr)
+				}
+				if string(b) != original {
+					t.Fatalf("original credentials overwritten after %s failed: sign-in error = %v", fault, err)
+				}
+				if err == nil || !strings.Contains(err.Error(), blocked) {
+					t.Fatalf("sign-in error = %v, want an error naming %s", err, blocked)
+				}
+			})
+		})
+	}
+}
+
 // Valid JSON can still be unreadable as saved accounts. On a cold start,
 // adding an account must keep those original credentials in a backup.
 func TestUnreadLoginsFieldTypesKeptBeforeWrite(t *testing.T) {