diff --git a/docs/subsystems/providers-accounts.md b/docs/subsystems/providers-accounts.md index 3c965406..53a88f5c 100644 --- a/docs/subsystems/providers-accounts.md +++ b/docs/subsystems/providers-accounts.md @@ -24,7 +24,7 @@ keys and accounts per request is in [Gateway routing and fallback](gateway-routi | Subscription accounts | `Accounts` reads the credentials each agent keeps on disk or in the macOS Keychain; nothing is stored twice. A built-in that has moved to its plugin is dropped (`Moved`), and plugin accounts are placed in its slot (`placeMoved`). `Sign` adds the account's own auth, then the user's `Headers`. | [`account.go`](../../internal/provider/account.go) | | Token refresh | Each account refreshes through its vendor. For example, `codexToken` refreshes within 5 minutes of expiry and writes the rotated tokens back into `~/.codex/auth.json` for Codex CLI. A 400 or 401 from the vendor's refresh endpoint means it was refused (`refreshFailed`). | [`account.go`](../../internal/provider/account.go) | | Saved accounts | `logins.json` remembers every Codex and Claude Code account seen signed in. A switch moves the credentials into the agent's own store and saves the ones it replaces. A saved account that is on (`SetLoginOn`) stands behind the agent's own. Its tokens stay in `logins.json`. | [`logins.go`](../../internal/provider/logins.go), [`logins_on.go`](../../internal/provider/logins_on.go) | -| Sign-in in magpie | `StartSignIn` runs the vendor's own browser OAuth with its client and takes the tokens at a callback on this machine. A browser that can't reach that callback (magpie on a server or in Docker) ends on a page that won't load; its address, pasted (`SubmitSignInCallback`), goes through the same callback handler (`pastedCallback`). A plugin's browser sign-in (`auto`) whose page names a port on this machine to come back to takes its pasted address too: magpie opens it there on 127.0.0.1, for the plugin's own listener (`pluginCallback`, `loopbackPorts`); Command Code's page posts its key from script, so it offers the API key way instead. A Claude account is signed in by Claude Code itself. | [`signin.go`](../../internal/provider/signin.go), [`claude_signin.go`](../../internal/provider/claude_signin.go), [`pluginsignin_paste.go`](../../internal/provider/pluginsignin_paste.go) | +| Sign-in in magpie | `StartSignIn` runs the vendor's own browser OAuth with its client and takes the tokens at a callback on this machine. A browser that can't reach that callback (magpie on a server or in Docker) ends on a page that won't load; its address, pasted (`SubmitSignInCallback`), goes through the same callback handler (`pastedCallback`). A plugin's browser sign-in (`auto`) whose page names a port on this machine to come back to takes its pasted address too: magpie opens it there on 127.0.0.1, for the plugin's own listener (`pluginCallback`, `loopbackPorts`); Command Code's page posts its key from script, so it offers the API key way instead. A Claude account is signed in by Claude Code itself. Signing in brings back an account removed from magpie (`ShowAccount`). A sign-in started in magpie (`StartSignIn`, `StartPluginSignIn`) reads as done (`SignInStatus`, `WaitSignIn`) only after `finish` has run `ShowAccount` for its account, so the window, which opens the account as soon as it sees done, finds it listed. If providers.json can't be written then, the account stays hidden and the sign-in still ends done. Signing in to a plugin's provider from the command line (`pluginLogin`, with a key or the browser) brings it back too. It also takes off the mark a sign-in its vendor turned away left on the account (`Lapsed`) and shows again the agent's own account removed in magpie (`clearPluginLapse`). The window's plugin sign-in does the same with the browser (`pluginDone`, then `finish`) or with a key (`PluginAPIKey`); a key and the command line share these steps in `PluginSignedIn`. | [`signin.go`](../../internal/provider/signin.go), [`claude_signin.go`](../../internal/provider/claude_signin.go), [`pluginsignin.go`](../../internal/provider/pluginsignin.go), [`pluginsignin_paste.go`](../../internal/provider/pluginsignin_paste.go), [`plugins_cli.go`](../../plugins_cli.go) | | Used-up switch | When the account Codex or Claude Code is signed in to has used its allowance, the agent is signed in to the next account that is on and has room (#208). | [`codex_switch.go`](../../internal/provider/codex_switch.go) | | Routing groups | User-made groups from the Routing view, and auto-found groups of the same model across providers (derived, never stored). Members are `provider/model[:effort]`. A group can nest another (`group/`); `SaveGroup` refuses loops. `Match` takes a glob or `re:` pattern (#766). | [`group.go`](../../internal/provider/group.go) | | Allowance | `Quotas` asks every vendor at once: subscription windows (`SubscriptionUsage`), key-bought plans (`PlanQuotas`) and key balances (`KeyBalances`). The Usage page, `magpie quota --json` and `GET /v1/magpie/quotas` read it. A single card can be read again on its own (#840). A remote magpie's cards come from what it has kept (`CachedCards`, `GET /v1/magpie/quotas/cards`) and are never read there for this computer except by a card's refresh (`POST /v1/magpie/quotas/refresh`, throttled per card). | [`quotas.go`](../../internal/provider/quotas.go), [`subscription_usage.go`](../../internal/provider/subscription_usage.go), [`planquota.go`](../../internal/provider/planquota.go), [`balance.go`](../../internal/provider/balance.go), [`quota_refresh.go`](../../internal/provider/quota_refresh.go), [`remote_quotas.go`](../../internal/provider/remote_quotas.go) | diff --git a/internal/provider/pluginsignin.go b/internal/provider/pluginsignin.go index 1f514631..04011630 100644 --- a/internal/provider/pluginsignin.go +++ b/internal/provider/pluginsignin.go @@ -169,9 +169,19 @@ func PluginAPIKey(ctx context.Context, id string, method int, inputs map[string] if err != nil { return "", err } + return PluginSignedIn(saved), nil +} + +// PluginSignedIn finishes a sign-in to a plugin's provider that magpie +// doesn't follow, a key or the command line's own, once the plugin has +// saved the account: as for one it follows (pluginDone), the account's +// lapsed mark goes and, removed from magpie, it comes back. It gives the +// provider's magpie id. +func PluginSignedIn(saved plugin.Saved) string { clearPluginLapse(saved) - _ = ShowAccount(PluginID(saved.Provider)) - return PluginID(saved.Provider), nil + id := PluginID(saved.Provider) + _ = ShowAccount(id) + return id } // pluginUsing is whether the account signed in to is the one in use, the diff --git a/internal/provider/signin.go b/internal/provider/signin.go index 4a2edd29..6cae114e 100644 --- a/internal/provider/signin.go +++ b/internal/provider/signin.go @@ -91,7 +91,10 @@ type signInFlow struct { // claimed is a callback being traded for the account: the browser's own // or a pasted address, whichever came first claimed bool - done chan struct{} + // finished is an outcome being recorded: the first finish's, which for a + // sign-in done reads as done once the account is shown again + finished bool + done chan struct{} } var signIns = struct { @@ -520,7 +523,7 @@ func (p *pastedReply) Write(b []byte) (int, error) { func (s *signInFlow) claim() bool { s.mu.Lock() defer s.mu.Unlock() - if s.st.State != "waiting" || s.claimed { + if s.st.State != "waiting" || s.claimed || s.finished { return false } s.claimed = true @@ -550,21 +553,27 @@ func (s *signInFlow) status() SignInState { return s.st } -// finish records the outcome once and lets the callback server go. +// finish records the outcome once and lets the callback server go. A +// sign-in done reads as done only once its account is shown again, so the +// window, seeing it done, finds the account listed. func (s *signInFlow) finish(out SignInState) bool { s.mu.Lock() - if s.st.State != "waiting" && s.st.State != "installing" { + if s.finished { s.mu.Unlock() return false } + s.finished = true + if out.State == "done" { + agent := s.st.Agent + s.mu.Unlock() + // signing in again brings back an account removed from magpie + _ = ShowAccount(agent) + s.mu.Lock() + } out.ID, out.Agent, out.URL, out.Code = s.st.ID, s.st.Agent, s.st.URL, s.st.Code s.st = out stop, srv := s.stop, s.srv s.mu.Unlock() - if out.State == "done" { - // signing in again brings back an account removed from magpie - _ = ShowAccount(out.Agent) - } close(s.done) if stop != nil { stop() diff --git a/internal/provider/signin_removed_test.go b/internal/provider/signin_removed_test.go new file mode 100644 index 00000000..a3710a68 --- /dev/null +++ b/internal/provider/signin_removed_test.go @@ -0,0 +1,217 @@ +package provider + +import ( + "context" + "encoding/json" + "net" + "net/http" + "net/http/httptest" + "os/exec" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/yetone/magpie/internal/catalog" + "github.com/yetone/magpie/internal/plugin" +) + +// Signing in to an account removed from magpie shows it again before the +// sign-in reads as done, so the window, seeing it done, finds the account +// listed rather than saying it can't list it (TestClaudeTeamSeatBesidePersonal +// found it missing on macOS in #1046's CI). Each way a sign-in finishes is +// checked: magpie's own callback (Codex), an agent's login command run by +// magpie (Claude) and a plugin's. +func TestSignInDoneOnceTheAccountIsBack(t *testing.T) { + for _, c := range []struct { + agent string + // setUp signs in to agent, which is then removed from magpie + setUp func(t *testing.T) + // signIn starts the sign-in again, and finish does what the + // browser, or the code its page shows, does then + signIn func(t *testing.T) SignInState + finish func(t *testing.T, st SignInState) + }{{ + agent: "codex", + setUp: fakeCodexSignIn, + signIn: func(t *testing.T) SignInState { + st, err := StartSignIn("codex") + if err != nil { + t.Fatal(err) + } + return st + }, + finish: func(t *testing.T, st SignInState) { finishInBrowser(t, st, "code") }, + }, { + agent: "claude", + setUp: func(t *testing.T) { + home := claudeHome(t) + noAnthropic(t) + fakeClaudeLogin(t, fakeClaudeAccount{email: "new@example.com", name: "New", plan: "max", refresh: "sk-ant-ort01-new"}, "", true) + claudeSignIn(t, home, time.Now().Add(time.Hour)) + writeFile(t, filepath.Join(home, ".claude.json"), map[string]any{"oauthAccount": map[string]any{"emailAddress": "old@example.com"}}) + }, + signIn: func(t *testing.T) SignInState { + st, err := StartSignIn("claude") + if err != nil { + t.Fatal(err) + } + return st + }, + finish: func(t *testing.T, st SignInState) { finishInBrowser(t, st, "the-code") }, + }, { + agent: "fakeco", + setUp: func(t *testing.T) { + bun, err := exec.LookPath("bun") + if err != nil { + t.Skip("no bun on PATH") + } + claudeHome(t) + t.Setenv("MAGPIE_BUN", bun) + t.Cleanup(plugin.Settle) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + abs, _ := filepath.Abs("../plugin/testdata/fake/index.js") + if _, err := plugin.Add(ctx, abs); err != nil { + t.Fatal(err) + } + if _, err := plugin.Providers(ctx); err != nil { + t.Fatal(err) + } + st, err := StartPluginSignIn("fakeco", 1, map[string]string{"where": "work", "team": "blue"}) + if err != nil { + t.Fatal(err) + } + if err := SubmitSignInCallback(st.ID, "good"); err != nil { + t.Fatal(err) + } + }, + signIn: func(t *testing.T) SignInState { + st, err := StartPluginSignIn("fakeco", 1, map[string]string{"where": "work", "team": "blue"}) + if err != nil { + t.Fatal(err) + } + return st + }, + finish: func(t *testing.T, st SignInState) { + if err := SubmitSignInCallback(st.ID, "good"); err != nil { + t.Fatal(err) + } + }, + }} { + t.Run(c.agent, func(t *testing.T) { + c.setUp(t) + if err := Delete(c.agent); err != nil { + t.Fatal(err) + } + if _, ok := find(All(), c.agent); ok { + t.Fatal("removed account still listed") + } + + // what the sign-in reads as the moment its account is shown again + var mu sync.Mutex + id, back := "", "" + was := catalog.Changed + catalog.Changed = func() { + mu.Lock() + defer mu.Unlock() + if id != "" && back == "" && !hiddenAccount(c.agent) { + st, _ := SignInStatus(id) + back = st.State + } + } + t.Cleanup(func() { catalog.Changed = was }) + + st := c.signIn(t) + mu.Lock() + id = st.ID + mu.Unlock() + c.finish(t, st) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + if st, err := WaitSignIn(ctx, st.ID); err != nil || st.State != "done" { + t.Fatalf("sign-in %+v, %v", st, err) + } + mu.Lock() + got := back + mu.Unlock() + switch got { + case "": + t.Fatal("signing in didn't show the removed account again") + case "done": + t.Fatal("the sign-in read as done before its account was shown again: the window, seeing it done, can't list it") + } + if _, ok := find(All(), c.agent); !ok { + t.Fatal("the account isn't listed after its sign-in") + } + }) + } +} + +// A sign-in canceled while its account is shown again, as the window still +// offers Cancel until it reads as done, ends done all the same: the account +// was signed in to and saved already, and a sign-in records one outcome. +func TestSignInCanceledWhileTheAccountComesBack(t *testing.T) { + fakeCodexSignIn(t) + if err := Delete("codex"); err != nil { + t.Fatal(err) + } + var mu sync.Mutex + id, canceled := "", false + was := catalog.Changed + catalog.Changed = func() { + mu.Lock() + defer mu.Unlock() + if id != "" && !canceled && !hiddenAccount("codex") { + canceled = true + CancelSignIn(id) + } + } + t.Cleanup(func() { catalog.Changed = was }) + + st, err := StartSignIn("codex") + if err != nil { + t.Fatal(err) + } + mu.Lock() + id = st.ID + mu.Unlock() + finishInBrowser(t, st, "code") + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + if st, err := WaitSignIn(ctx, st.ID); err != nil || st.State != "done" || st.User != "me@example.com" { + t.Fatalf("sign-in %+v, %v", st, err) + } + mu.Lock() + shown := canceled + mu.Unlock() + if !shown { + t.Fatal("signing in didn't show the removed account again") + } + if _, ok := find(All(), "codex"); !ok { + t.Fatal("the account isn't listed after its sign-in") + } +} + +// fakeCodexSignIn signs in to Codex (me@example.com, personal Pro, +// workspace acct-1) and points its sign-in at a token server that signs in +// to that account again, coming back to a free port. +func fakeCodexSignIn(t *testing.T) { + t.Helper() + signIn(t) + fake := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "id_token": fakeJWT(map[string]any{"email": "me@example.com", + "https://api.openai.com/auth": map[string]any{"chatgpt_plan_type": "pro", "chatgpt_account_id": "acct-1"}}), + "access_token": fakeJWT(map[string]any{"exp": float64(time.Now().Add(time.Hour).Unix())}), + "refresh_token": "r2", + }) + })) + t.Cleanup(fake.Close) + oldTok, oldAddr := codexTokenURL, codexCallbackAddr + t.Cleanup(func() { codexTokenURL, codexCallbackAddr = oldTok, oldAddr }) + codexTokenURL = fake.URL + ln, _ := net.Listen("tcp", "127.0.0.1:0") + codexCallbackAddr = ln.Addr().String() + ln.Close() +} diff --git a/plugin_login_cli_test.go b/plugin_login_cli_test.go new file mode 100644 index 00000000..b3d396c5 --- /dev/null +++ b/plugin_login_cli_test.go @@ -0,0 +1,107 @@ +package main + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + "time" + + "github.com/yetone/magpie/internal/plugin" + "github.com/yetone/magpie/internal/provider" + "github.com/yetone/magpie/internal/testenv" +) + +// magpie accounts add, signing in to a plugin's provider removed from +// magpie, brings it back, as the window's sign-in and a built-in's do. +func TestAccountsAddBringsBackARemovedPlugin(t *testing.T) { + ctx := fakeCoHome(t) + if _, err := provider.PluginAPIKey(ctx, "fakeco", 0, nil, "sk-one-key-1111"); err != nil { + t.Fatal(err) + } + if err := provider.Delete("fakeco"); err != nil { + t.Fatal(err) + } + if _, err := provider.Find("fakeco"); err == nil { + t.Fatal("removed provider still listed") + } + // the API key way (1), then the key + piped(t, "1", "sk-two-key-2222") + if out, err := said(t, func() error { return accountsCmd([]string{"accounts", "add", "fakeco"}) }); err != nil { + t.Fatalf("%v: %s", err, out) + } + if _, err := provider.Find("fakeco"); err != nil { + t.Fatalf("signed in again, but not listed: %v", err) + } +} + +// magpie plugin login, signing in again to an account whose sign-in the +// vendor turned away, takes its lapsed mark off, as the window's sign-in +// does, rather than the account saying "sign in again" until a request or +// a usage read goes through. +func TestPluginLoginTakesTheLapseOff(t *testing.T) { + ctx := fakeCoHome(t) + noBrowser(t) + // the browser's way (2), at work (2) on the team "gone", whose usage + // says its sign-in has expired, then the code the page shows + login := func() { + t.Helper() + piped(t, "2", "2", "gone", "good") + if out, err := said(t, func() error { return pluginCmd([]string{"plugin", "login", "fakeco"}) }); err != nil { + t.Fatalf("%v: %s", err, out) + } + } + login() + provider.LoginUsage(ctx, "fakeco") + if ls := provider.Logins("fakeco"); len(ls) != 1 || ls[0].Lapsed == "" { + t.Fatalf("the vendor's refusal didn't mark the account: %+v", ls) + } + login() + if ls := provider.Logins("fakeco"); len(ls) != 1 || ls[0].Lapsed != "" { + t.Fatalf("signed in again, but still lapsed: %+v", ls) + } +} + +// fakeCoHome is a HOME of the test's own with the plugin host's test +// plugin (FakeCo) added, as TestAccountsOfAPlugin sets one up. +func fakeCoHome(t *testing.T) context.Context { + t.Helper() + bun, err := exec.LookPath("bun") + if err != nil { + t.Skip("no bun on PATH") + } + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) + t.Setenv("CLAUDE_CONFIG_DIR", filepath.Join(home, ".claude")) + t.Setenv("MAGPIE_BUN", bun) + t.Cleanup(plugin.Settle) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + t.Cleanup(cancel) + abs, _ := filepath.Abs("internal/plugin/testdata/fake/index.js") + if _, err := plugin.Add(ctx, abs); err != nil { + t.Fatal(err) + } + if _, err := plugin.Providers(ctx); err != nil { + t.Fatal(err) + } + return ctx +} + +// noBrowser has the sign-in pages magpie opens go nowhere: an open (macOS) +// and an xdg-open (Linux) that do nothing come first on PATH. +func noBrowser(t *testing.T) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("rundll32 opens the browser on Windows") + } + dir := t.TempDir() + for _, name := range []string{"open", "xdg-open"} { + testenv.Program(t, filepath.Join(dir, name), "#!/bin/sh\nexit 0\n") + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} diff --git a/plugins_cli.go b/plugins_cli.go index 60f11dc9..d57d48c0 100644 --- a/plugins_cli.go +++ b/plugins_cli.go @@ -439,7 +439,10 @@ func pluginLogin(ctx context.Context, name, method string) error { return err } } - fmt.Println(green.Render("✓"), "signed in to", pp.Name, muted.Render("· its models are "+provider.PluginID(saved.Provider)+"/")) + // as the window's sign-in does: its lapsed mark goes and, removed from + // magpie, it comes back + id := provider.PluginSignedIn(saved) + fmt.Println(green.Render("✓"), "signed in to", pp.Name, muted.Render("· its models are "+id+"/")) return nil }