accounts: on Linux an account put first is read first at once, and on Windows a file being replaced isn't read as none (goal: 插件订阅 provider 跟内置的所有逻辑都要一模一样, run on Windows and Linux) — the provider tests run on the Windows test box and in WSL found two bugs of magpie's own files. Linux stamps a file's time from a clock a tick (a few milliseconds) coarse, and filememo took same time and size for the same file: two accounts swapped (Devin, Grok, WorkBuddy, ZCode, Claude…) read back in the old order, so magpie kept using the account the user had just moved down, and a moved-back plugin's accounts read as before. A file written within the last two seconds is now also compared by its bytes. On Windows a file being renamed over refuses to open for a moment ("Access is denied"), and one held open refuses to be renamed over: plugin-auth.json, plugins.json and the plugin providers read as nothing (a usage fetch right after a move showed no cards), logins.json failed to save, and host.js, reading plugin-auth.json as {}, would have written the other accounts away on the next sign-in. New internal/steady waits such a read or rename out (half a second at most, Windows only); filememo, edit.WriteAtomic, writePrivate, Kiro's writes and the plugin store go through it, host.js retries its read and rename, and the plugin store writes its files by a rename. Tests: USERPROFILE beside HOME in the test homes, the fake devin (a shell script) skipped on Windows, plugin-auth.json's mode bits unchecked there. TestReadSameSizeSameTick fails on Linux and TestRenameOverAFileHeldOpen/TestReadAFileHeldOpen on Windows without the change. go vet, go build -tags nogui (darwin, GOOS=linux), GOOS=windows go build, go test -tags nogui ./... pass; on Linux (WSL) the provider and plugin packages pass whole (13 failed before), on Windows the plugin package and the move/plugin/Grok/Devin/usage tests pass 10 times over (TestMoveToPlugin failed one run in a few before); 15 built-in tests that assume Unix (mode bits, Keychain, shell-script CLIs) still fail there, as before.

This commit is contained in:
yetone committed 2026-10-01 13:27:12 +08:00
1 parent 22b6ad19f9
commit 7dd809002b
15 files changed
+240 -21

No files matched your search

+3 -1
View File
@@ -12,6 +12,8 @@ import (
"path/filepath"
"strings"
"time"
"github.com/yetone/magpie/internal/steady"
)
// Read returns the file contents, or (nil, nil) when the file does not exist.
@@ -79,7 +81,7 @@ func WriteAtomic(path string, data []byte) error {
cleanup()
return err
}
if err := os.Rename(tmpName, path); err != nil {
if err := steady.Rename(tmpName, path); err != nil {
cleanup()
return err
}
+30 -4
View File
@@ -3,24 +3,37 @@
package filememo
import (
"bytes"
"os"
"sync"
"time"
"github.com/yetone/magpie/internal/steady"
)
type entry struct {
mod time.Time
size int64
v any
// the bytes parsed, while the file's time is too recent to tell a
// second write by: Linux stamps a file from a clock a tick (a few
// milliseconds) coarse, so two writes of one size inside a tick (two
// accounts swapped) leave the same time and size
raw []byte
}
// settled is how old a file's time must be for any later write to stamp
// a later one, many ticks of the coarsest clock.
const settled = 2 * time.Second
var (
mu sync.Mutex
seen = map[string]entry{}
)
// Read is parse of the file at path, from what it gave last time when the
// file has the same size and time as then. kind tells apart two parses of
// file has the same size and time as then (and, written just now, the same
// bytes). kind tells apart two parses of
// one file. What it returns is shared: the caller must not change it.
func Read[T any](kind, path string, parse func([]byte) (T, error)) (T, error) {
var zero T
@@ -32,19 +45,32 @@ func Read[T any](kind, path string, parse func([]byte) (T, error)) (T, error) {
mu.Lock()
e, ok := seen[key]
mu.Unlock()
if ok && e.mod.Equal(fi.ModTime()) && e.size == fi.Size() {
if ok && e.mod.Equal(fi.ModTime()) && e.size == fi.Size() && e.raw == nil {
return e.v.(T), nil
}
b, err := os.ReadFile(path)
b, err := steady.ReadFile(path)
if err != nil {
return zero, err
}
if ok && e.mod.Equal(fi.ModTime()) && bytes.Equal(b, e.raw) {
if time.Since(e.mod) > settled {
mu.Lock()
e.raw = nil
seen[key] = e
mu.Unlock()
}
return e.v.(T), nil
}
v, err := parse(b)
if err != nil {
return zero, err
}
mu.Lock()
seen[key] = entry{mod: fi.ModTime(), size: fi.Size(), v: v}
e = entry{mod: fi.ModTime(), size: fi.Size(), v: v}
if time.Since(e.mod) <= settled {
e.raw = b
}
seen[key] = e
mu.Unlock()
return v, nil
}
+20
View File
@@ -1,6 +1,7 @@
package filememo
import (
"fmt"
"os"
"path/filepath"
"testing"
@@ -30,3 +31,22 @@ func TestRead(t *testing.T) {
t.Fatal("a file gone is an error")
}
}
// two writes of one size inside one tick of the file system's clock (Linux
// stamps a file's time from a clock a few milliseconds coarse) leave the
// same time and size: the second is still read, not the first's parse
func TestReadSameSizeSameTick(t *testing.T) {
p := filepath.Join(t.TempDir(), "f")
parse := func(b []byte) (string, error) { return string(b), nil }
for i := range 200 {
a, b := fmt.Sprintf("a%03d,b", i), fmt.Sprintf("b%03d,a", i)
os.WriteFile(p, []byte(a), 0o600)
if v, _ := Read("tick", p, parse); v != a {
t.Fatalf("read %q, want %q", v, a)
}
os.WriteFile(p, []byte(b), 0o600)
if v, _ := Read("tick", p, parse); v != b {
t.Fatalf("read %q after writing %q: the first write's parse", v, b)
}
}
}
+27 -2
View File
@@ -120,8 +120,21 @@ globalThis.fetch = Object.assign(function fetch(input, init) {
// ---- auth.json ---------------------------------------------------------------
function readAuth() {
let text
for (let i = 0; ; i++) {
try {
text = fs.readFileSync(authPath, "utf8")
break
} catch (e) {
if (e?.code === "ENOENT") return {}
// Windows refuses a file being renamed over for a moment: read as
// none, the next setAuth would write the others' accounts away
if (i >= 50) throw e
pause(10)
}
}
try {
const v = JSON.parse(fs.readFileSync(authPath, "utf8"))
const v = JSON.parse(text)
return v && typeof v === "object" ? v : {}
} catch {
return {}
@@ -132,7 +145,19 @@ function writeAuth(all) {
fs.mkdirSync(path.dirname(authPath), { recursive: true })
const tmp = authPath + ".tmp-" + process.pid
fs.writeFileSync(tmp, JSON.stringify(all, null, 2) + "\n", { mode: 0o600 })
fs.renameSync(tmp, authPath)
// and a file held open by a reader refuses to be renamed over
for (let i = 0; ; i++) {
try {
return fs.renameSync(tmp, authPath)
} catch (e) {
if (i >= 50 || !["EPERM", "EACCES", "EBUSY"].includes(e?.code)) throw e
pause(10)
}
}
}
function pause(ms) {
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms)
}
function setAuth(key, info) {
+2 -1
View File
@@ -12,6 +12,7 @@ import (
"os/exec"
"path/filepath"
"reflect"
"runtime"
"slices"
"strings"
"sync"
@@ -121,7 +122,7 @@ func TestFakePlugin(t *testing.T) {
if saved["fakeco"]["type"] != "oauth" || saved["fakeco"]["refresh"] != "r-blue" || saved["fakeco"]["accountId"] != "blue@fake" {
t.Fatalf("saved %v", saved)
}
if fi, _ := os.Stat(AuthPath()); fi.Mode().Perm() != 0o600 {
if fi, _ := os.Stat(AuthPath()); runtime.GOOS != "windows" && fi.Mode().Perm() != 0o600 { // Windows keeps no mode bits
t.Fatalf("plugin-auth.json is %v", fi.Mode().Perm())
}
if !SignedIn("fakeco") {
+8 -7
View File
@@ -13,6 +13,7 @@ import (
"time"
"github.com/yetone/magpie/internal/settings"
"github.com/yetone/magpie/internal/steady"
)
// Method is a way a plugin signs in: "oauth" (a browser, then a code
@@ -133,7 +134,7 @@ func Providers(ctx context.Context) ([]Provider, error) {
provCache, provGood = ps, true
provMu.Unlock()
if b, err := json.Marshal(ps); err == nil {
_ = os.WriteFile(providersPath(), b, 0o600)
_ = writeWhole(providersPath(), b)
}
return ps, nil
}
@@ -145,7 +146,7 @@ func Providers(ctx context.Context) ([]Provider, error) {
// why, rather than going as if it were removed.
func keepUnloaded(ps, last []Provider) []Provider {
if last == nil {
if b, err := os.ReadFile(providersPath()); err == nil {
if b, err := steady.ReadFile(providersPath()); err == nil {
_ = json.Unmarshal(b, &last)
}
}
@@ -197,7 +198,7 @@ func Cached() []Provider {
good := provGood
provMu.Unlock()
if ps == nil {
if b, err := os.ReadFile(providersPath()); err == nil {
if b, err := steady.ReadFile(providersPath()); err == nil {
_ = json.Unmarshal(b, &ps)
}
}
@@ -302,7 +303,7 @@ func firstNonEmpty(ss ...string) string {
func readAuth() map[string]storedAuth {
var m map[string]storedAuth
if b, err := os.ReadFile(AuthPath()); err == nil {
if b, err := steady.ReadFile(AuthPath()); err == nil {
_ = json.Unmarshal(b, &m)
}
return m
@@ -433,7 +434,7 @@ func SignOut(ctx context.Context, provider, account string) error {
return Call(ctx, "signOut", map[string]any{"provider": provider, "account": account}, nil)
}
var m map[string]json.RawMessage
b, err := os.ReadFile(AuthPath())
b, err := steady.ReadFile(AuthPath())
if err != nil {
return nil
}
@@ -446,7 +447,7 @@ func SignOut(ctx context.Context, provider, account string) error {
}
}
b, _ = json.MarshalIndent(m, "", " ")
if err := os.WriteFile(AuthPath(), append(b, '\n'), 0o600); err != nil {
if err := writeWhole(AuthPath(), append(b, '\n')); err != nil {
return err
}
changed()
@@ -540,7 +541,7 @@ func Check(ctx context.Context, provider, account string) (Checked, error) {
// Auths are provider's sign-ins as plugin-auth.json keeps them, by key.
func Auths(provider string) map[string]map[string]any {
var m map[string]map[string]any
if b, err := os.ReadFile(AuthPath()); err == nil {
if b, err := steady.ReadFile(AuthPath()); err == nil {
_ = json.Unmarshal(b, &m)
}
out := map[string]map[string]any{}
+24 -2
View File
@@ -33,6 +33,7 @@ import (
"github.com/yetone/magpie/internal/netproxy"
"github.com/yetone/magpie/internal/proc"
"github.com/yetone/magpie/internal/settings"
"github.com/yetone/magpie/internal/steady"
)
// Entry is one plugin the user added.
@@ -69,12 +70,33 @@ func listPath() string { return filepath.Join(settings.Dir(), "plugins.json") }
// Load reads plugins.json.
func Load() List {
var l List
if b, err := os.ReadFile(listPath()); err == nil {
if b, err := steady.ReadFile(listPath()); err == nil {
_ = json.Unmarshal(b, &l)
}
return l
}
// writeWhole writes b to p by a rename, so a magpie or the host reading
// p meanwhile reads the old file or the new one, never one half-written;
// read back with steady.ReadFile, which waits out the rename on Windows.
func writeWhole(p string, b []byte) error {
f, err := os.CreateTemp(filepath.Dir(p), filepath.Base(p)+".*")
if err != nil {
return err
}
_, err = f.Write(b)
if cerr := f.Close(); err == nil {
err = cerr
}
if err == nil {
err = steady.Rename(f.Name(), p)
}
if err != nil {
os.Remove(f.Name())
}
return err
}
func save(l List) error {
b, err := json.MarshalIndent(l, "", " ")
if err != nil {
@@ -83,7 +105,7 @@ func save(l List) error {
if err := os.MkdirAll(settings.Dir(), 0o700); err != nil {
return err
}
if err := os.WriteFile(listPath(), append(b, '\n'), 0o600); err != nil {
if err := writeWhole(listPath(), append(b, '\n')); err != nil {
return err
}
listSeen.Lock()
+2
View File
@@ -222,6 +222,8 @@ func claudeHome(t *testing.T) string {
isolate(t)
home := t.TempDir()
t.Setenv("HOME", home)
// Windows finds the home in USERPROFILE
t.Setenv("USERPROFILE", home)
t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config"))
t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache"))
forgetClaudeCredential()
+9
View File
@@ -174,6 +174,9 @@ func TestDevinCredentials(t *testing.T) {
}
func TestDevinSignIn(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("the fake devin is a shell script")
}
home := claudeHome(t)
t.Setenv("XDG_DATA_HOME", filepath.Join(home, "data"))
@@ -242,6 +245,9 @@ func TestDevinSignIn(t *testing.T) {
// own: it gets a home of magpie's, the CLI's file is left alone, and the
// gateway can use either.
func TestDevinSeveralAccounts(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("the fake devin is a shell script")
}
home := claudeHome(t)
data := filepath.Join(home, "data")
t.Setenv("XDG_DATA_HOME", data)
@@ -362,6 +368,9 @@ const devinUnreachable = `printf '%s\n' 'Logged in (via Devin).' '' 'User / team
// either.
func fakeDevin(t *testing.T, exe string) {
t.Helper()
if runtime.GOOS == "windows" {
t.Skip("the fake devin is a shell script")
}
old := DevinExecutable
DevinExecutable = func() string { return exe }
t.Cleanup(func() {
+1
View File
@@ -79,6 +79,7 @@ func googleSandbox(t *testing.T, f *fakeGoogle) {
t.Helper()
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home) // Windows's home
t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config"))
t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache"))
srv := httptest.NewServer(f)
+2 -1
View File
@@ -33,6 +33,7 @@ import (
"github.com/yetone/magpie/internal/catalog"
"github.com/yetone/magpie/internal/edit"
"github.com/yetone/magpie/internal/proc"
"github.com/yetone/magpie/internal/steady"
)
// KiroExecutable finds kiro-cli, which refreshes its own sign-in; a var so
@@ -464,7 +465,7 @@ func writeFileAtomic(path string, b []byte) error {
if err := os.WriteFile(tmp, b, 0o600); err != nil {
return err
}
return os.Rename(tmp, path)
return steady.Rename(tmp, path)
}
// kiroPost posts to a sign-in endpoint.
+2 -1
View File
@@ -24,6 +24,7 @@ import (
"github.com/yetone/magpie/internal/edit"
"github.com/yetone/magpie/internal/filememo"
"github.com/yetone/magpie/internal/plugin"
"github.com/yetone/magpie/internal/steady"
)
// Login is a remembered subscription account, without its secrets.
@@ -148,7 +149,7 @@ func writePrivate(path string, b []byte) error {
if err := tmp.Close(); err != nil {
return err
}
return os.Rename(tmp.Name(), path)
return steady.Rename(tmp.Name(), path)
}
func upsertLogin(ls []savedLogin, l savedLogin) []savedLogin {
+4 -2
View File
@@ -196,13 +196,15 @@ func TestMoveToPlugin(t *testing.T) {
}
// a built-in with no usage card of its own (Devin) keeps the plugin's
cards := 0
for _, q := range fetchSubscriptionUsage() {
qs := fetchSubscriptionUsage()
for _, q := range qs {
if q.Provider == "fakeco" {
cards++
}
}
if cards == 0 {
t.Fatal("the moved plugin's accounts show no usage")
t.Fatalf("the moved plugin's accounts show no usage: moved %v, plugins %d, logins %d, cards %+v",
Moved("fakeco"), len(plugin.Cached()), len(pluginUsageLogins(mustPlugin(t))), qs)
}
// the user reorders on the plugin, and it renews a's token; back, the
+46
View File
@@ -0,0 +1,46 @@
// Package steady reads and replaces files that another process may be
// replacing at that moment. On Windows a file being renamed over refuses
// to open, and one held open refuses to be renamed over, both for a
// moment ("Access is denied"): read as none, a file of accounts is no
// accounts, and the next write leaves them out. Elsewhere a rename is
// atomic and these are os.ReadFile and os.Rename.
package steady
import (
"errors"
"io/fs"
"os"
"runtime"
"time"
)
// tries and pause bound the wait to half a second: what holds the file is
// another read or rename, done in milliseconds.
const (
tries = 50
pause = 10 * time.Millisecond
)
func retry(f func() error) error {
err := f()
for i := 0; err != nil && runtime.GOOS == "windows" && !errors.Is(err, fs.ErrNotExist) && i < tries; i++ {
time.Sleep(pause)
err = f()
}
return err
}
// ReadFile is os.ReadFile, waiting out a file being replaced.
func ReadFile(p string) ([]byte, error) {
var b []byte
err := retry(func() (err error) {
b, err = os.ReadFile(p)
return err
})
return b, err
}
// Rename is os.Rename, waiting out a reader holding newpath open.
func Rename(oldpath, newpath string) error {
return retry(func() error { return os.Rename(oldpath, newpath) })
}
+60
View File
@@ -0,0 +1,60 @@
package steady
import (
"os"
"path/filepath"
"syscall"
"testing"
"time"
)
// a reader holding the file without FILE_SHARE_DELETE, as other programs
// open it, for a moment: os.Rename over it is refused, Rename waits it out
func TestRenameOverAFileHeldOpen(t *testing.T) {
dir := t.TempDir()
p, tmp := filepath.Join(dir, "logins.json"), filepath.Join(dir, "logins.json.tmp")
os.WriteFile(p, []byte("old"), 0o600)
os.WriteFile(tmp, []byte("new"), 0o600)
name, _ := syscall.UTF16PtrFromString(p)
h, err := syscall.CreateFile(name, syscall.GENERIC_READ, syscall.FILE_SHARE_READ, nil, syscall.OPEN_EXISTING, 0, 0)
if err != nil {
t.Fatal(err)
}
if err := os.Rename(tmp, p); err == nil {
t.Fatal("renamed over a file held open: nothing to wait out")
}
go func() {
time.Sleep(100 * time.Millisecond)
syscall.CloseHandle(h)
}()
if err := Rename(tmp, p); err != nil {
t.Fatal(err)
}
if b, err := ReadFile(p); err != nil || string(b) != "new" {
t.Fatalf("read %q, %v", b, err)
}
}
// and a file held so refuses to be read until let go
func TestReadAFileHeldOpen(t *testing.T) {
p := filepath.Join(t.TempDir(), "plugin-auth.json")
os.WriteFile(p, []byte("accounts"), 0o600)
name, _ := syscall.UTF16PtrFromString(p)
h, err := syscall.CreateFile(name, syscall.GENERIC_WRITE, 0, nil, syscall.OPEN_EXISTING, 0, 0)
if err != nil {
t.Fatal(err)
}
if _, err := os.ReadFile(p); err == nil {
t.Fatal("read a file held unshared: nothing to wait out")
}
go func() {
time.Sleep(100 * time.Millisecond)
syscall.CloseHandle(h)
}()
if b, err := ReadFile(p); err != nil || string(b) != "accounts" {
t.Fatalf("read %q, %v", b, err)
}
if _, err := ReadFile(filepath.Join(filepath.Dir(p), "none")); !os.IsNotExist(err) {
t.Fatalf("a file not there: %v", err)
}
}