From 4a07b0c5f9c3f76b61ae63cb28b86466d33daea2 Mon Sep 17 00:00:00 2001 From: Haogao Gu Date: Sat, 3 Oct 2026 13:31:04 -0400 Subject: [PATCH 1/3] Handle Claude sign-in failures without timed retries --- internal/gateway/claude_auth_test.go | 61 ++++++++ internal/gateway/claude_subscription.go | 9 ++ internal/gateway/fallback.go | 2 +- internal/gateway/gateway.go | 9 ++ internal/gateway/routing.go | 4 + internal/gui/assets/app.js | 25 ++- internal/gui/assets/i18n.js | 8 + internal/gui/assets/routing.js | 8 +- .../gui/tests/claude-auth-status.test.cjs | 106 +++++++++++++ internal/provider/claude_auth.go | 82 ++++++++++ internal/provider/claude_auth_test.go | 148 ++++++++++++++++++ internal/provider/claude_dirs.go | 38 +++-- internal/provider/claude_probe.go | 4 +- internal/provider/logins.go | 15 +- internal/provider/logins_on.go | 5 +- internal/provider/test.go | 1 + 16 files changed, 502 insertions(+), 23 deletions(-) create mode 100644 internal/gateway/claude_auth_test.go create mode 100644 internal/gui/tests/claude-auth-status.test.cjs create mode 100644 internal/provider/claude_auth.go create mode 100644 internal/provider/claude_auth_test.go diff --git a/internal/gateway/claude_auth_test.go b/internal/gateway/claude_auth_test.go new file mode 100644 index 000000000..4fe04663b --- /dev/null +++ b/internal/gateway/claude_auth_test.go @@ -0,0 +1,61 @@ +package gateway + +import ( + "context" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestClaudeAuthFailureFallsBackWithoutRetryingTheLogin(t *testing.T) { + claudeMadeFirst(t, false) + s := New() + t.Cleanup(s.subscription.abortAll) + logfile := filepath.Join(t.TempDir(), "calls") + script := `#!/bin/sh +case "$1" in auth) exit 1;; esac +creds="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/.credentials.json" +while read -r line; do + tok=$(grep -o 'tok-[a-z]*' "$creds" | head -1) + echo "$tok" >> '` + logfile + `' + if [ "$tok" = "tok-a" ]; then + echo '{"type":"result","is_error":true,"result":"Failed to authenticate: OAuth session expired and could not be refreshed"}' + else + echo '{"type":"stream_event","event":{"type":"message_start","message":{"id":"m","model":"claude-sonnet-5","usage":{"input_tokens":1}}}}' + echo '{"type":"stream_event","event":{"type":"content_block_delta","index":0,"delta":{"type":"text_delta","text":"healthy account"}}}' + echo '{"type":"stream_event","event":{"type":"message_delta","delta":{"stop_reason":"end_turn"},"usage":{"output_tokens":3}}}' + echo '{"type":"stream_event","event":{"type":"message_stop"}}' + echo '{"type":"result","is_error":false,"result":""}' + fi +done +` + binary := strings.Split(os.Getenv("PATH"), string(os.PathListSeparator))[0] + if err := os.WriteFile(filepath.Join(binary, "claude"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + for _, text := range []string{"first", "second"} { + body := `{"model":"claude/claude-sonnet-5","max_tokens":100,"messages":[{"role":"user","content":"` + text + `"}]}` + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest("POST", "/v1/messages", strings.NewReader(body))) + if rec.Code != 200 || !strings.Contains(rec.Body.String(), "healthy account") { + t.Fatalf("%s: %d %s", text, rec.Code, rec.Body.String()) + } + } + calls, _ := os.ReadFile(logfile) + if strings.Count(string(calls), "tok-a\n") != 1 { + t.Fatalf("rejected login retried: %s", calls) + } + var first *Route + for _, route := range s.Trace(context.Background(), 0, 0).Routes { + if len(route.Tries) == 2 { + r := route + first = &r + } + } + if first == nil || first.Tries[0].Fail != failAuth || first.Tries[0].Rest != nil || + !strings.Contains(first.Tries[0].Error, "OAuth session expired") { + t.Fatalf("authentication reason lost or shown as a timed rest: %+v", first) + } +} diff --git a/internal/gateway/claude_subscription.go b/internal/gateway/claude_subscription.go index 01ea7a2e3..cd0730c03 100644 --- a/internal/gateway/claude_subscription.go +++ b/internal/gateway/claude_subscription.go @@ -130,6 +130,8 @@ type subscriptionRun struct { // --effort) or was told since (setEffort); "" is Claude Code's own effort string + loginVersion string + // told is the conversation as the client had it in its last request // here (historyKey): tool results are the run's while the client's // conversation goes on from that one. @@ -325,6 +327,9 @@ func (b *subscriptionBridge) start(ctx context.Context, req *Request, model, con } run := &subscriptionRun{bridge: b, token: token, model: model, cmd: cmd, tmp: tmp, schema: len(req.Schema) > 0, pending: map[string]chan mcpToolResult{}, stdin: stdin, owner: owner, effort: req.Effort} + if fields := strings.Split(owner, "\x00"); len(fields) > 1 { + run.loginVersion = provider.ClaudeLoginVersion(fields[1]) + } // A caller may abandon a turn after receiving tool_use. Do not leave the // parked Claude process and MCP request alive forever. run.timer = time.AfterFunc(30*time.Minute, run.abort) @@ -851,6 +856,10 @@ func (r *subscriptionRun) readOutput(rd io.Reader) { text += ": " + strings.Join(envelope.Errors, "; ") } } + if fields := strings.Split(r.owner, "\x00"); len(fields) > 1 && + !(len(fields) > 2 && fields[2] == ownHome && provider.ClaudeCodeMovedOff(fields[1])) { + provider.NoteClaudeSignInFailure(fields[1], r.loginVersion, text) + } r.emit(Event{Kind: KError, Text: text, Status: envelope.APIErrorStatus, Code: errKind, RequestID: reqID}) r.endSegment() case waiting && len(envelope.StructuredOutput) > 0 && string(envelope.StructuredOutput) != "null": diff --git a/internal/gateway/fallback.go b/internal/gateway/fallback.go index 5d111d0a1..ddedd39f1 100644 --- a/internal/gateway/fallback.go +++ b/internal/gateway/fallback.go @@ -133,7 +133,7 @@ func perKeyBarred(p provider.Provider, model string, from provider.Protocol) (ou var all []candidate // the account the agent is signed in to, unless the user paused // it for the others on (#263) - if len(also) == 0 || !p.OwnPaused() { + if p.Account.SignInError() == "" && (len(also) == 0 || !p.OwnPaused()) { all = append(all, candidate{p: p, model: model, rest: p.ID}) } for i, q := range also { diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index 976f029e4..50be415c6 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -1649,6 +1649,15 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request, from provider.Pro skipped = append(skipped, c.label()+": "+call.Error) continue } + if !last && hw.failed() && failure(hw.code(), hw.errBody()) == failAuth { + if other == nil { + other = &Try{Status: call.Status, Error: call.Error} + } + try.Fail = failAuth + s.trace.update(tr, func(t *Route) { t.Tries[len(t.Tries)-1] = try }) + skipped = append(skipped, c.label()+": "+call.Error) + continue + } if wait, ok := passing(hw.code(), hw.header, hw.errBody(), again); ok && !last && hw.failed() && spentAfter(cands[i+1:]) { // the others left are out of their allowance (Discord, waroy: a // Codex account run out, Grok busy a moment): this one is the diff --git a/internal/gateway/routing.go b/internal/gateway/routing.go index 27e8b7449..bf05eaa9e 100644 --- a/internal/gateway/routing.go +++ b/internal/gateway/routing.go @@ -129,6 +129,7 @@ const ( failQuota = "quota" failRate = "rate" failOther = "other" + failAuth = "auth" // failCanceled: the agent went away before the answer came failCanceled = "canceled" // failForeign: the conversation's reasoning was sealed by another @@ -168,6 +169,9 @@ var proxyDown = regexp.MustCompile(`proxyconnect |socks connect `) // failure says why a reply failed. func failure(status int, body []byte) string { + if status >= 400 && provider.ClaudeSignInRequired(string(body)) { + return failAuth + } if status == http.StatusBadGateway && proxyDown.Match(body) { return failProxy } diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index fd8366ce6..caca1303b 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -6311,7 +6311,8 @@ function renderEndpoints(p, src) { s.className = "res " + (x.ok ? "ok" : "bad"); s.replaceChildren(); s.append(svg(x.ok ? CHECK : "M4.5 4.5l7 7M11.5 4.5l-7 7", 10, 2)); - s.append(el("span", "", x.ok ? ledTook(x.ms) : x.status ? `${x.status} · ${x.error}` : x.error)); + const result = x.ok ? ledTook(x.ms) : x.status ? `${x.status} · ${x.error}` : x.error; + s.append(el("span", "", x.account ? t("Tested {user}: {result}", { user: x.account, result }) : result)); s.title = x.ok ? t("model {model}", { model: x.model }) : x.error; } } catch (e) { for (const s of Object.values(slots)) { s.className = "res"; s.textContent = ""; } status(e.message, "err"); } @@ -7857,7 +7858,7 @@ function renderAccounts(a, p) { const sub = subOf(a.agent); const list = el("div", "accts"); const ls = loginsInOrder(a, p); - const several = ls.filter((l) => (l.active && !l.paused) || l.on).length > 1; + const several = ls.filter((l) => !l.lapsed && ((l.active && !l.paused) || l.on)).length > 1; // kept signed in to one of the user's choosing (#524), the first is the // first in use in the order, which Make first sets without a sign-in const kept = keptLogin(p); @@ -7871,18 +7872,21 @@ function renderAccounts(a, p) { // the account Claude Code or Codex is signed in to can be paused while // another is on: the gateway passes over it, the agent staying signed in // to it (#263) - const pausable = (a.agent === "claude" || a.agent === "codex") && ls.some((l) => !l.active && l.on); + const pausable = (a.agent === "claude" || a.agent === "codex") && ls.some((l) => !l.lapsed && !l.active && l.on); const quota = loginUsageOf(a.agent); // the first, which magpie signed the agent out of while it was spent: // it is signed back in once it has room (#408) const back = ls.find((l) => l.returns && !l.active); for (const l of ls) { - const on = !l.paused && (l.active || l.on); + const on = !l.lapsed && !l.paused && (l.active || l.on); const row = el("div", "acc" + (on ? " in-use" : " off") + (l.user === justAdded ? " new" : "")); row.dataset.accountId = l.user; const dot = el("button", "dot tick"); if (on) dot.append(svg(CHECK, 10, 2.2)); - if (l.active && (pausable || l.paused)) { + if (l.lapsed) { + dot.disabled = true; + dot.title = t("Sign in again to use this account"); + } else if (l.active && (pausable || l.paused)) { dot.title = l.paused ? t("Resume: the gateway uses this account first again") : t("Pause: the gateway uses the other accounts, {agent} stays signed in to this one", { agent: a.agentName }); dot.onclick = () => accountAction("login/" + (l.paused ? "on" : "off"), { agent: a.agent, user: l.user }); } else if (l.active) { @@ -7897,7 +7901,14 @@ function renderAccounts(a, p) { const [amPill, amBox] = ls.length > 1 || accountModelsOf(p, l.user).length ? accountModels(p, l.user, false, l.user) : []; if (amPill) row.append(amPill); row.append(el("span", "grow")); - if (l.active && kept && l.user !== firstUser) { + if (l.lapsed) { + row.append(el("span", "using", t("Sign-in required"))); + const again = el("button", "text", t("Sign in again")); + again.onclick = () => startSignIn(a.agent); + const forget = el("button", "text quiet", t("Remove")); + forget.onclick = () => accountAction("login/forget", { agent: a.agent, user: l.user }, t("{user} removed", { user: l.user })); + row.append(again, forget); + } else if (l.active && kept && l.user !== firstUser) { // signed in to, kept so, and tried at its place in the order const signed = el("span", "using", l.paused ? t("Paused") : t("Signed in")); signed.title = t("{agent} is kept signed in to this account; requests through magpie go to the accounts in their order", { agent: a.agentName }); @@ -7935,7 +7946,7 @@ function renderAccounts(a, p) { row.append(forget, use); } } - row.append(accountQuota(l.lapsed ? { [l.user]: { error: l.lapsed } } : quota, l.user)); + row.append(accountQuota(l.lapsed ? { [l.user]: { error: t(l.lapsed) } } : quota, l.user)); row.classList.add("with-aq"); // not :has(.aq), which Safari 15.0 lacks (#220) if (amBox) row.append(amBox); list.append(row); diff --git a/internal/gui/assets/i18n.js b/internal/gui/assets/i18n.js index cfb50e5cf..53b806569 100644 --- a/internal/gui/assets/i18n.js +++ b/internal/gui/assets/i18n.js @@ -928,6 +928,14 @@ const I18N = { "The agents given it use magpie's sign-in": "分配到它的 agent 都用 magpie 的登录", "Sign-in ran out": "登录已失效", "Sign in again": "重新登录", + "Sign-in required": "需要重新登录", + "sign-in required": "需要重新登录", + "Sign in again to use this account": "重新登录后才能使用此账号", + "Claude Code is no longer signed in; sign in again in magpie": "Claude Code 已没有有效登录，请在 magpie 中重新登录", + "Claude Code could not authenticate this account; sign in again in magpie": "Claude Code 无法认证此账号，请在 magpie 中重新登录", + "Tested {user}: {result}": "已测试 {user}：{result}", + "{who} could not authenticate. Sign in again in magpie; this login is not retried. The request went to {next}.": "{who} 认证失败。请在 magpie 中重新登录；当前登录不再重试。请求已转给 {next}。", + "{who} could not authenticate. Sign in again in magpie; this login is not retried. No other account could answer.": "{who} 认证失败。请在 magpie 中重新登录；当前登录不再重试。没有其他账号可以回答。", "Open it to sign in again": "打开它重新登录", "Signed in to {name} — the agents given it use magpie's sign-in": "已登录 {name}——分配到它的 agent 都用 magpie 的登录", "Signed out of {name} — the agents are given the server's own address again": "已退出 {name}——agent 重新使用服务器自己的地址", diff --git a/internal/gui/assets/routing.js b/internal/gui/assets/routing.js index c2fa1bdb5..a3390f7fa 100644 --- a/internal/gui/assets/routing.js +++ b/internal/gui/assets/routing.js @@ -168,7 +168,7 @@ // (WorkBuddy's credits, #659): "355 / 500 credits · 71% used" const quota = (w, used, left, vars) => (w.limit > 0 ? quotaCount(w) + " · " : "") + t(quotaLeft ? left : used, { n: pct(share(w)), ...vars }); const fill = (w) => Math.max(0, Math.min(100, share(w))) + "%"; - const FAIL = { rate: "rate limited", credit: "out of credit", quota: "quota used up", other: "failed", canceled: "canceled", foreign: "another account's reasoning", floor: "reply too short", verify: "needs verification", refused: "refused (safety filter)", shape: "request not understood", proxy: "proxy not reachable", effort: "reasoning effort not in its plan", overflow: "too long for its model" }; + const FAIL = { rate: "rate limited", credit: "out of credit", quota: "quota used up", other: "failed", auth: "sign-in required", canceled: "canceled", foreign: "another account's reasoning", floor: "reply too short", verify: "needs verification", refused: "refused (safety filter)", shape: "request not understood", proxy: "proxy not reachable", effort: "reasoning effort not in its plan", overflow: "too long for its model" }; const failWord = (why) => t(FAIL[why] || "failed"); const API = { anthropic: "Anthropic", chat: "OpenAI", responses: "OpenAI Responses", gemini: "Gemini" }; const MODES = { @@ -551,6 +551,12 @@ { who: name, status: tr.status, account: tr.reset.who, agent }); if (tr.fail === "canceled") return t("{agent} canceled the request while {who} was answering: nobody failed, so nobody rests and nobody else is asked.", { who: name, agent }); + if (tr.fail === "auth") { + const next = r.tries[i + 1], nw = next && tried(r, next); + return next + ? t("{who} could not authenticate. Sign in again in magpie; this login is not retried. The request went to {next}.", { who: name, next: nw ? who(nw) : t("the next") }) + : t("{who} could not authenticate. Sign in again in magpie; this login is not retried. No other account could answer.", { who: name }); + } if (tr.fail === "foreign") return t("{who} couldn't read the reasoning another account wrote earlier in this conversation, so it is asked again without it, before any of the reply reaches {agent}.", { who: name, agent }); if (tr.fail === "floor") diff --git a/internal/gui/tests/claude-auth-status.test.cjs b/internal/gui/tests/claude-auth-status.test.cjs new file mode 100644 index 000000000..8c1d6ceff --- /dev/null +++ b/internal/gui/tests/claude-auth-status.test.cjs @@ -0,0 +1,106 @@ +const assert = require("node:assert/strict"); +const fs = require("node:fs/promises"); +const path = require("node:path"); +const { test } = require("node:test"); +const { chromium, webkit } = require("playwright"); + +const assets = path.resolve(__dirname, "../assets"); +const lapse = "Claude Code could not authenticate this account; sign in again in magpie"; +const error = "Failed to authenticate: OAuth session expired and could not be refreshed"; +const at = new Date().toISOString(); +const expired = { id: "claude@expired", provider: "claude", name: "Claude Code", kind: "account", who: "expired@example.com", model: "claude-opus-5-5" }; +const healthy = { ...expired, id: "claude", who: "healthy@example.com" }; +const routes = [ + { id: 103, seq: 103, time: at, agent: "codex", model: "claude/claude-opus-5-5", provider: "claude", + order: [expired, healthy], tries: [ + { id: expired.id, model: expired.model, done: true, status: 502, error, fail: "auth", ms: 300 }, + { id: healthy.id, model: healthy.model, done: true, status: 200, ms: 300 }, + ], done: true, status: 200 }, + { id: 102, seq: 102, time: at, agent: "codex", model: "claude/claude-opus-5-5", provider: "claude", + order: [expired], tries: [{ id: expired.id, model: expired.model, done: true, status: 502, error, fail: "auth", ms: 300 }], + done: true, status: 502, error }, +]; +const provider = { + id: "claude", name: "Claude Code", icon: "claudecode-color", anthropic: "https://api.example.test", + chat: "", responses: "", catalog: "", models: [{ id: "claude-opus-5-5", on: true }], + agents: [], fallback: [], headers: {}, keyList: [], proxy: "", + account: { agent: "claude", agentName: "Claude Code", user: healthy.who, plan: "max", logins: [ + { user: healthy.who, plan: "max", active: true, on: true }, + { user: expired.who, plan: "enterprise", on: true, lapsed: lapse }, + ] }, +}; + +function serve(lang) { + return async (route) => { + const url = new URL(route.request().url()); + const json = (data) => route.fulfill({ json: data }); + if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs={lang:"${lang}",theme:"light",web:true};` }); + if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" }); + if (url.pathname === "/api/state") return json({ agents: [{ id: "codex", name: "Codex", fields: [] }], profiles: [], settings: { lang, theme: "light" } }); + if (url.pathname === "/api/providers") return json({ providers: [provider], presets: [], excluded: [], gateway: { running: true, window: true } }); + if (url.pathname === "/api/provider/test") return json({ results: [{ protocol: "anthropic", ok: true, ms: 300, model: healthy.model, account: healthy.who }], provider }); + if (url.pathname === "/api/gateway/trace") { + if (url.searchParams.get("wait")) await new Promise((r) => setTimeout(r, 20e3)); + return json({ mine: true, now: at, seq: 103, totals: { requests: 2, errors: 1 }, routes }); + } + if (url.pathname === "/api/gateway/history") return json({ cut: false, days: [], routes: [] }); + if (url.pathname === "/api/login/usage") return json({}); + if (url.pathname === "/api/usage/quotas") return json([]); + if (url.pathname === "/api/groups") return json({ groups: [] }); + if (url.pathname === "/api/plugins") return json({ plugins: [] }); + if (url.pathname.startsWith("/api/")) return json({}); + const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname); + const contentType = { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)]; + try { await route.fulfill({ body: await fs.readFile(file), contentType }); } + catch { await route.fulfill({ status: 404, body: "" }); } + }; +} + +for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) { + for (const lang of ["en", "zh"]) { + test(`${engine} ${lang}: Claude auth failures require sign-in instead of a timed retry`, async (t) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + t.after(() => browser.close()); + const page = await (await browser.newContext({ viewport: { width: 1100, height: 800 }, reducedMotion: "reduce" })).newPage(); + const errors = []; + page.on("pageerror", (e) => errors.push(e.message)); + page.setDefaultTimeout(5000); + await page.route("**/*", serve(lang)); + await page.goto("http://magpie.test/?view=routing"); + const steps = page.locator(".rt-steps"); + await steps.getByText(lang === "zh" ? /当前登录不再重试/ : /this login is not retried/).waitFor(); + let story = await steps.textContent(); + assert(story.includes(error), "the original CLI error remains visible"); + assert(story.includes(healthy.who), "the fallback account is named"); + assert(!/backoff|rests|休息/.test(story), "authentication is not a cooldown"); + if (process.env.ARTIFACT_DIR) { + await fs.mkdir(process.env.ARTIFACT_DIR, { recursive: true }); + await steps.screenshot({ path: path.join(process.env.ARTIFACT_DIR, `${engine}-${lang}-claude-auth-route.png`) }); + } + await page.locator(".rt-req").nth(1).click(); + await steps.getByText(lang === "zh" ? /没有其他账号可以回答/ : /No other account could answer/).waitFor(); + assert((await steps.textContent()).includes(error)); + + await page.goto("http://magpie.test/?view=providers"); + await page.locator(".row.provider", { hasText: "Claude Code" }).first().click(); + const row = page.locator(`.editor .acc[data-account-id="${expired.who}"]`); + await row.waitFor(); + assert.equal(await row.locator(".dot").isDisabled(), true); + assert.equal(await row.locator(".dot svg").count(), 0, "an expired account is not shown in use"); + assert.equal(await row.locator(".using").textContent(), lang === "zh" ? "需要重新登录" : "Sign-in required"); + await row.getByRole("button", { name: lang === "zh" ? "重新登录" : "Sign in again", exact: true }).waitFor(); + assert.equal(await row.getByRole("button", { name: lang === "zh" ? "设为首选" : "Make first", exact: true }).count(), 0); + if (process.env.ARTIFACT_DIR) { + await page.locator(".editor .accts").screenshot({ path: path.join(process.env.ARTIFACT_DIR, `${engine}-${lang}-claude-auth-accounts.png`) }); + } + await page.locator(".editor .eps").getByRole("button", { name: lang === "zh" ? "测试" : "Test", exact: true }).click(); + await page.locator(".editor .res").getByText(new RegExp(healthy.who.replaceAll(".", "\\."))).waitFor(); + assert(!(await page.locator(".editor .res").textContent()).includes(expired.who), "test success identifies only the account tested"); + assert.deepEqual(errors, []); + if (process.env.ARTIFACT_DIR) { + await fs.mkdir(process.env.ARTIFACT_DIR, { recursive: true }); + await page.screenshot({ path: path.join(process.env.ARTIFACT_DIR, `${engine}-${lang}-claude-auth.png`) }); + } + }); + } +} diff --git a/internal/provider/claude_auth.go b/internal/provider/claude_auth.go new file mode 100644 index 000000000..20728dd49 --- /dev/null +++ b/internal/provider/claude_auth.go @@ -0,0 +1,82 @@ +package provider + +import ( + "crypto/sha256" + "fmt" + "strings" +) + +const claudeAuthLapse = "Claude Code could not authenticate this account; sign in again in magpie" + +// ClaudeSignInRequired recognizes a CLI refusal that needs a new login, +// not a network failure or another process temporarily holding the refresh lock. +func ClaudeSignInRequired(message string) bool { + for _, text := range []string{ + "OAuth session expired and could not be refreshed", + "OAuth access token has been revoked", + "OAuth token revoked", + claudeAuthLapse, + claudeLogoutLapse, + legacyClaudeLogoutLapse, + } { + if strings.Contains(message, text) { + return true + } + } + return false +} + +func claudeLoginVersion(l savedLogin) string { + c, ok := parseClaudeCredentials(l.Auth) + if !ok { + return "" + } + return fmt.Sprintf("%x", sha256.Sum256([]byte(c.OAuth.AccessToken+"\x00"+c.OAuth.RefreshToken))) +} + +// ClaudeLoginVersion identifies the saved login a CLI run starts with. +// An error from an older run must not invalidate a newly authorized login. +func ClaudeLoginVersion(user string) string { + loginsMu.Lock() + defer loginsMu.Unlock() + for _, l := range readLogins() { + if l.Agent == "claude" && strings.EqualFold(l.User, user) { + return claudeLoginVersion(l) + } + } + return "" +} + +// NoteClaudeSignInFailure records a terminal authentication refusal for +// this login only. The original CLI error remains in the request trace. +func NoteClaudeSignInFailure(user, version, message string) { + if version == "" || !ClaudeSignInRequired(message) { + return + } + loginsMu.Lock() + defer loginsMu.Unlock() + ls := readLogins() + for i := range ls { + if ls[i].Agent == "claude" && strings.EqualFold(ls[i].User, user) && + claudeLoginVersion(ls[i]) == version && ls[i].Lapsed == "" { + ls[i].Lapsed = claudeAuthLapse + _ = writeLogins(ls) + return + } + } +} + +// SignInError is a saved Claude account's terminal login state. +func (a *Account) SignInError() string { + if a == nil || a.Agent != "claude" { + return "" + } + loginsMu.Lock() + defer loginsMu.Unlock() + for _, l := range readLogins() { + if l.Agent == "claude" && strings.EqualFold(l.User, a.User) && l.Lapsed != "" { + return claudeSignedOut(l) + } + } + return "" +} diff --git a/internal/provider/claude_auth_test.go b/internal/provider/claude_auth_test.go new file mode 100644 index 000000000..451d7218f --- /dev/null +++ b/internal/provider/claude_auth_test.go @@ -0,0 +1,148 @@ +package provider + +import ( + "context" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +func claudeAuthFixture(t *testing.T, lapse string) (Provider, savedLogin) { + t.Helper() + testHome := claudeHome(t) + noAnthropic(t) + t.Setenv("CLAUDE_CONFIG_DIR", "") + claudeSignIn(t, testHome, time.Now().Add(time.Hour)) + writeFile(t, filepath.Join(testHome, ".claude.json"), map[string]any{ + "oauthAccount": map[string]any{"emailAddress": "own@example.com"}, + }) + side := savedLogin{Agent: "claude", User: "side@example.com", On: true, Plan: "max", Seen: time.Now().Add(-time.Hour), Lapsed: lapse, + Auth: mustJSONRaw(t, map[string]any{"claudeAiOauth": map[string]any{ + "accessToken": "old-access", "refreshToken": "old-refresh", + "expiresAt": time.Now().Add(time.Hour).UnixMilli(), "subscriptionType": "max", + }})} + writeFile(t, loginsPath(), []savedLogin{side}) + forgetAccountCaches() + p, ok := find(All(), "claude") + if !ok { + t.Fatal("own Claude account missing") + } + return p, side +} + +func TestClaudeLapsedLoginIsNotRoutedOrRestored(t *testing.T) { + p, side := claudeAuthFixture(t, legacyClaudeLogoutLapse) + if also := p.AlsoOn(); len(also) != 0 { + t.Fatalf("lapsed account still routed: %+v", also) + } + if _, err := claudeSavedDir(side.User); err == nil || !strings.Contains(err.Error(), "sign in again") { + t.Fatalf("lapsed login restored: %v", err) + } + if _, err := os.Stat(claudeAccountDir(side.User)); !os.IsNotExist(err) { + t.Fatalf("a directory was created for the lapsed login: %v", err) + } + for _, l := range Logins("claude") { + if l.User == side.User && (l.Lapsed != claudeLogoutLapse || strings.Contains(l.Lapsed, "/logout")) { + t.Fatalf("absence blamed on logout: %+v", l) + } + } +} + +func TestClaudeAuthFailureRequiresNewLoginAndIgnoresOldRuns(t *testing.T) { + p, side := claudeAuthFixture(t, "") + oldVersion := ClaudeLoginVersion(side.User) + NoteClaudeSignInFailure(side.User, oldVersion, "Failed to authenticate: OAuth session expired and could not be refreshed") + if len(p.AlsoOn()) != 0 { + t.Fatal("authentication failure was not removed from routing") + } + if err := SwitchLogin("claude", side.User); err == nil { + t.Fatal("switching restored a rejected login") + } + c, _ := parseClaudeCredentials(side.Auth) + c.OAuth.AccessToken, c.OAuth.RefreshToken = "new-access", "new-refresh" + side.Auth, _ = c.marshal() + if _, err := addLogin(side); err != nil { + t.Fatal(err) + } + NoteClaudeSignInFailure(side.User, oldVersion, "OAuth token revoked") + also := p.AlsoOn() + if len(also) != 1 { + t.Fatalf("an old run invalidated the new login: %+v", also) + } + if _, _, err := also[0].Account.Token(context.Background()); err != nil { + t.Fatalf("new login could not be used: %v", err) + } +} + +func TestClaudeClearedKeychainDoesNotFallBackToStaleFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("a shell script stands in for the keychain") + } + _, side := claudeAuthFixture(t, "") + dir, err := claudeSavedDir(side.User) + if err != nil { + t.Fatal(err) + } + bin := t.TempDir() + tombstone := filepath.Join(bin, "cleared.json") + writeFile(t, tombstone, map[string]any{"claudeAiOauth": map[string]any{ + "accessToken": "", "refreshToken": "", "expiresAt": 0, "subscriptionType": "max", + }}) + script := "#!/bin/sh\ncat '" + tombstone + "'\n" + if err := os.WriteFile(filepath.Join(bin, "security"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + claudeKeychain = true + if _, err := claudeSavedDir(side.User); err == nil { + t.Fatal("cleared keychain fell back to the old credentials file") + } + b, _ := os.ReadFile(filepath.Join(dir, ".credentials.json")) + if !strings.Contains(string(b), "old-refresh") { + t.Fatal("the stale file was rewritten rather than refusing it") + } + for _, l := range Logins("claude") { + if l.User == side.User && l.Lapsed == "" { + t.Fatal("cleared saved login was not marked as needing sign-in") + } + } +} + +func TestClaudeSignInRequiredDoesNotBlameTemporaryFailures(t *testing.T) { + for _, tc := range []struct { + message string + want bool + }{ + {"Failed to authenticate: OAuth session expired and could not be refreshed", true}, + {"OAuth access token has been revoked.", true}, + {"Failed to refresh OAuth token: another Claude Code process is refreshing it or exited mid-refresh", false}, + {"OAuth token refresh failed (HTTP 503)", false}, + {"Failed to authenticate: network connection timed out", false}, + {"You've hit your session limit", false}, + } { + if got := ClaudeSignInRequired(tc.message); got != tc.want { + t.Errorf("%q: got %v, want %v", tc.message, got, tc.want) + } + } +} + +func TestClaudeProbeReportsTheAccountAndMarksItsFailedLogin(t *testing.T) { + p, side := claudeAuthFixture(t, "") + old := claudeCLIProbe + t.Cleanup(func() { claudeCLIProbe = old }) + claudeCLIProbe = func(context.Context, string, string) error { + return errors.New("Failed to authenticate: OAuth session expired and could not be refreshed") + } + saved := p.AlsoOn()[0] + result := saved.testClaude(context.Background(), "claude-sonnet-5") + if result.OK || result.Account != side.User || !strings.Contains(result.Error, "OAuth session expired") { + t.Fatalf("failed test lost the account or original error: %+v", result) + } + if len(p.AlsoOn()) != 0 { + t.Fatal("the login that failed its connection test is still routed") + } +} diff --git a/internal/provider/claude_dirs.go b/internal/provider/claude_dirs.go index 1cb3f162b..9abff3a42 100644 --- a/internal/provider/claude_dirs.go +++ b/internal/provider/claude_dirs.go @@ -49,8 +49,10 @@ func readClaudeDir(dir string) (claudeCredentials, bool) { out, err := proc.Command("security", "find-generic-password", "-s", claudeDirService(dir), "-a", claudeKeychainAccount(), "-w").Output() if err == nil { b, _ := keychainText(bytes.TrimSpace(out)) - if c, ok := parseClaudeCredentials(b); ok { - return c, true + if c, ok := parseClaudeCredentials(b); c.raw != nil { + // An existing, cleared keychain record is authoritative: + // a file left beside it must not resurrect the rejected login. + return c, ok } } } @@ -95,14 +97,25 @@ func claudeSavedDir(user string) (string, error) { if i < 0 { return "", fmt.Errorf("no saved claude account %q", user) } + if ls[i].Lapsed != "" { + return "", errors.New(claudeSignedOut(ls[i])) + } dir := claudeAccountDir(user) - if c, ok := readClaudeDir(dir); ok { + c, ok := readClaudeDir(dir) + if !ok && c.raw != nil { + ls[i].Lapsed = claudeAuthLapse + if err := writeLogins(ls); err != nil { + return "", err + } + return "", errors.New(claudeAuthLapse) + } + if ok { if changed, err := takeClaudeDir(&ls[i], c); err != nil || !changed { return dir, err } return dir, writeLogins(ls) } - c, ok := parseClaudeCredentials(ls[i].Auth) + c, ok = parseClaudeCredentials(ls[i].Auth) if !ok { return "", errors.New("the saved Claude sign-in of " + user + " is unreadable") } @@ -163,7 +176,7 @@ func claudeStandIn(ls []savedLogin) string { user, best := "", -1 var seen time.Time for _, l := range ls { - if l.Agent != "claude" || l.Held || l.Lapsed == claudeLogoutLapse { + if l.Agent != "claude" || l.Held || l.Lapsed != "" { continue } rank := 0 @@ -182,12 +195,13 @@ func claudeStandIn(ls []savedLogin) string { // claudeLogoutLapse is why the account Claude Code was signed in to can't // be used once Claude Code logged out. -const claudeLogoutLapse = "Claude Code's /logout signed it out (it revokes the sign-in it holds); sign in again" +const claudeLogoutLapse = "Claude Code is no longer signed in; sign in again in magpie" + +const legacyClaudeLogoutLapse = "Claude Code's /logout signed it out (it revokes the sign-in it holds); sign in again" // claudeLoggedOut marks the account Claude Code held, now that it is -// signed out, as lapsed: /logout revokes the refresh token Claude Code -// holds (POST /revoke, Claude Code 2.1.x's performLogout), and -// magpie's copy of that account is the same sign-in, so it is gone too. +// signed out, as lapsed. The absence of a login does not tell whether the +// user logged out or Claude Code cleared a rejected refresh token. // The accounts magpie keeps in config directories of their own are // sign-ins of their own and stay. It says whether ls changed. func claudeLoggedOut(ls []savedLogin) bool { @@ -207,6 +221,12 @@ func claudeLoggedOut(ls []savedLogin) bool { // claudeSignedOut is why a saved Claude account can't be used: its saved // sign-in is gone, and it has to be signed in again; "" when it has one. func claudeSignedOut(l savedLogin) string { + if l.Lapsed != "" { + if l.Lapsed == legacyClaudeLogoutLapse { + return claudeLogoutLapse + } + return l.Lapsed + } if _, ok := parseClaudeCredentials(l.Auth); ok { return "" } diff --git a/internal/provider/claude_probe.go b/internal/provider/claude_probe.go index 243fdf210..ccd1bc471 100644 --- a/internal/provider/claude_probe.go +++ b/internal/provider/claude_probe.go @@ -28,7 +28,7 @@ const claudeWait = time.Minute // testClaude is a Claude account's test of model, run by Claude Code. func (p Provider) testClaude(ctx context.Context, model string) Result { - r := Result{Protocol: Anthropic, Model: model} + r := Result{Protocol: Anthropic, Model: model, Account: p.Account.User} if model == "" { r.Error = "no model to try: expose one, or refresh the model list" return r @@ -45,9 +45,11 @@ func (p Provider) testClaude(ctx context.Context, model string) Result { return r } start := time.Now() + version := ClaudeLoginVersion(p.Account.User) err = claudeCLIProbe(ctx, dir, model) r.Millis = time.Since(start).Milliseconds() if err != nil { + NoteClaudeSignInFailure(p.Account.User, version, err.Error()) r.Error = err.Error() return r } diff --git a/internal/provider/logins.go b/internal/provider/logins.go index defbfe60d..58c24903d 100644 --- a/internal/provider/logins.go +++ b/internal/provider/logins.go @@ -179,6 +179,10 @@ func writePrivate(path string, b []byte) error { func upsertLogin(ls []savedLogin, l savedLogin) []savedLogin { for i := range ls { if sameLogin(ls[i], l) { + if l.Agent == "claude" && l.Lapsed == "" && ls[i].Lapsed != "" && + claudeLoginVersion(l) == claudeLoginVersion(ls[i]) { + l.Lapsed = ls[i].Lapsed + } l.On = l.On || ls[i].On l.Paused = l.Paused || ls[i].Paused l.Order = ls[i].Order @@ -595,10 +599,12 @@ func Logins(agent string) []Login { first := l.Agent == "claude" && strings.EqualFold(standIn, l.User) lg := Login{Agent: l.Agent, User: l.User, Plan: l.Plan, Seen: l.Seen, Active: using, On: using || first || l.On, Paused: (using || first) && pausedOwn(ls, l.Agent, l.User), first: first} + if l.Agent == "claude" { + lg.Lapsed = claudeSignedOut(l) + } if !using { - lg.Lapsed = l.Lapsed - if lg.Lapsed == "" && l.Agent == "claude" { - lg.Lapsed = claudeSignedOut(l) + if l.Agent != "claude" { + lg.Lapsed = l.Lapsed } lg.Returns = l.On && strings.EqualFold(back[l.Agent], l.User) } @@ -766,6 +772,9 @@ func switchSavedLogin(agent, user string) (from string, _ error) { return "", fmt.Errorf("no saved %s account %q", agent, user) } if agent == "claude" { + if target.Lapsed != "" { + return "", errors.New(claudeSignedOut(*target)) + } // as Claude Code keeps it, if it has run on the account beside the // one it is signed in to if c, ok := readClaudeDir(claudeAccountDir(target.User)); ok { diff --git a/internal/provider/logins_on.go b/internal/provider/logins_on.go index befb75feb..b3f1c08c2 100644 --- a/internal/provider/logins_on.go +++ b/internal/provider/logins_on.go @@ -170,7 +170,7 @@ func (p Provider) AlsoOn() []Provider { } var out []Provider for _, l := range Logins(p.Account.Agent) { - if l.Active || l.first || !l.On { + if l.Active || l.first || !l.On || l.Lapsed != "" { continue } agent, user := l.Agent, l.User @@ -199,6 +199,9 @@ func (p Provider) AlsoOn() []Provider { // own — for a Claude account, the config directory Claude Code runs on it // in; ok is false for the agent's own, which the agent signs itself. func (a *Account) Token(ctx context.Context) (tok string, ok bool, err error) { + if msg := a.SignInError(); msg != "" { + return "", false, errors.New(msg) + } if a == nil || a.token == nil { return "", false, nil } diff --git a/internal/provider/test.go b/internal/provider/test.go index 676687dcc..51c87373f 100644 --- a/internal/provider/test.go +++ b/internal/provider/test.go @@ -21,6 +21,7 @@ import ( // Result is what a probe of one endpoint came back with. type Result struct { + Account string `json:"account,omitempty"` Protocol Protocol `json:"protocol"` OK bool `json:"ok"` Status int `json:"status,omitempty"` From d1d7a9ff2f8e0d91db9fefc44facd26192e80efd Mon Sep 17 00:00:00 2001 From: Haogao Gu Date: Sat, 3 Oct 2026 15:13:01 -0400 Subject: [PATCH 2/3] Judge a Claude login by the credential it has now A refusal is kept for the credential it was made on (savedLogin.Refused) and holds only while the account still has that credential. One check, claudeSignedOut, decides whether a saved Claude login can be used, after syncClaudeDir has read what Claude Code keeps in the account's directory: a credential it refreshed to there is taken, and one it emptied (as Claude Code 2.1.x does after Anthropic answers invalid_grant) is refused. Token is the only gate: a saved account's directory, or the credential Claude Code itself holds for its own account. Lapsed accounts stay candidates, so the trace shows each one passed over and the agent is told to sign in again when none is left. This fixes four paths of the first version: a moment the own sign-in could not be read left it lapsed after it came back unchanged; a credential refreshed in the account's directory after a refusal was ignored; a switch could restore a copy Claude Code had emptied; and with every account refused the agent got "none of ... is ready" (404). --- internal/gateway/claude_auth_test.go | 89 +++++++++--- internal/gateway/claude_subscription.go | 6 + internal/gateway/fallback.go | 2 +- internal/gateway/gateway.go | 4 + internal/provider/claude_auth.go | 98 ++++++++++---- internal/provider/claude_auth_test.go | 173 ++++++++++++++++++------ internal/provider/claude_dirs.go | 99 +++++++++----- internal/provider/logins.go | 28 ++-- internal/provider/logins_on.go | 17 ++- 9 files changed, 369 insertions(+), 147 deletions(-) diff --git a/internal/gateway/claude_auth_test.go b/internal/gateway/claude_auth_test.go index 4fe04663b..666866d96 100644 --- a/internal/gateway/claude_auth_test.go +++ b/internal/gateway/claude_auth_test.go @@ -2,25 +2,33 @@ package gateway import ( "context" + "encoding/json" "net/http/httptest" "os" "path/filepath" + "slices" "strings" "testing" + + "github.com/yetone/magpie/internal/provider" ) -func TestClaudeAuthFailureFallsBackWithoutRetryingTheLogin(t *testing.T) { - claudeMadeFirst(t, false) - s := New() - t.Cleanup(s.subscription.abortAll) - logfile := filepath.Join(t.TempDir(), "calls") +// claudeRefuses has the fake Claude Code fail as Claude Code does on a +// sign-in Anthropic refused, on the accounts whose token is one of toks +// (all of them with none), and answer on the others. +func claudeRefuses(t *testing.T, calls string, toks ...string) { + t.Helper() + refused := `[ -z "` + strings.Join(toks, "") + `" ]` + for _, tok := range toks { + refused += ` || [ "$tok" = "` + tok + `" ]` + } script := `#!/bin/sh case "$1" in auth) exit 1;; esac creds="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/.credentials.json" while read -r line; do tok=$(grep -o 'tok-[a-z]*' "$creds" | head -1) - echo "$tok" >> '` + logfile + `' - if [ "$tok" = "tok-a" ]; then + echo "$tok" >> '` + calls + `' + if ` + refused + `; then echo '{"type":"result","is_error":true,"result":"Failed to authenticate: OAuth session expired and could not be refreshed"}' else echo '{"type":"stream_event","event":{"type":"message_start","message":{"id":"m","model":"claude-sonnet-5","usage":{"input_tokens":1}}}}' @@ -35,27 +43,64 @@ done if err := os.WriteFile(filepath.Join(binary, "claude"), []byte(script), 0o755); err != nil { t.Fatal(err) } +} + +func askClaudeModel(s *Server, text string) *httptest.ResponseRecorder { + body := `{"model":"claude/claude-sonnet-5","max_tokens":100,"messages":[{"role":"user","content":"` + text + `"}]}` + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest("POST", "/v1/messages", strings.NewReader(body))) + return rec +} + +// An account Anthropic refused goes to the next one at once, with no rest +// to wait out and then fail again: its sign-in is not run again. +func TestClaudeAuthFailureFallsBackWithoutRetryingTheLogin(t *testing.T) { + claudeMadeFirst(t, false) + s := New() + t.Cleanup(s.subscription.abortAll) + calls := filepath.Join(t.TempDir(), "calls") + claudeRefuses(t, calls, "tok-a") for _, text := range []string{"first", "second"} { - body := `{"model":"claude/claude-sonnet-5","max_tokens":100,"messages":[{"role":"user","content":"` + text + `"}]}` - rec := httptest.NewRecorder() - s.Handler().ServeHTTP(rec, httptest.NewRequest("POST", "/v1/messages", strings.NewReader(body))) - if rec.Code != 200 || !strings.Contains(rec.Body.String(), "healthy account") { + if rec := askClaudeModel(s, text); rec.Code != 200 || !strings.Contains(rec.Body.String(), "healthy account") { t.Fatalf("%s: %d %s", text, rec.Code, rec.Body.String()) } } - calls, _ := os.ReadFile(logfile) - if strings.Count(string(calls), "tok-a\n") != 1 { - t.Fatalf("rejected login retried: %s", calls) + if b, _ := os.ReadFile(calls); strings.Count(string(b), "tok-a\n") != 1 { + t.Fatalf("refused login run again: %s", b) } - var first *Route - for _, route := range s.Trace(context.Background(), 0, 0).Routes { - if len(route.Tries) == 2 { - r := route - first = &r + routes := s.Trace(context.Background(), 0, 0).Routes + slices.SortFunc(routes, func(a, b Route) int { return int(a.Seq - b.Seq) }) + for i, route := range routes { + if len(route.Tries) != 2 || route.Tries[0].Fail != failAuth || route.Tries[0].Rest != nil { + t.Fatalf("request %d: the refusal not told as one, or rested: %+v", i, route.Tries) } } - if first == nil || first.Tries[0].Fail != failAuth || first.Tries[0].Rest != nil || - !strings.Contains(first.Tries[0].Error, "OAuth session expired") { - t.Fatalf("authentication reason lost or shown as a timed rest: %+v", first) + if !strings.Contains(routes[0].Tries[0].Error, "OAuth session expired") || !strings.Contains(routes[1].Tries[0].Error, "sign in again") { + t.Fatalf("errors told: %q, %q", routes[0].Tries[0].Error, routes[1].Tries[0].Error) + } +} + +// With every account refused, the agent is told to sign in again, not that +// nothing is ready. +func TestClaudeAllRefusedSaysSignInAgain(t *testing.T) { + claudeMadeFirst(t, false) + loginFile := filepath.Join(filepath.Dir(provider.Path()), "logins.json") + var logins []map[string]any + raw, _ := os.ReadFile(loginFile) + if err := json.Unmarshal(raw, &logins); err != nil { + t.Fatal(err) + } + for _, l := range logins { + l["on"] = false + } + if err := os.WriteFile(loginFile, mustJSON(logins), 0o600); err != nil { + t.Fatal(err) + } + claudeRefuses(t, filepath.Join(t.TempDir(), "calls")) + s := New() + t.Cleanup(s.subscription.abortAll) + askClaudeModel(s, "first") + if rec := askClaudeModel(s, "second"); rec.Code == 200 || !strings.Contains(rec.Body.String(), "sign in again") { + t.Fatalf("not told to sign in again: %d %s", rec.Code, rec.Body.String()) } } diff --git a/internal/gateway/claude_subscription.go b/internal/gateway/claude_subscription.go index cd0730c03..b53a6c929 100644 --- a/internal/gateway/claude_subscription.go +++ b/internal/gateway/claude_subscription.go @@ -130,6 +130,9 @@ type subscriptionRun struct { // --effort) or was told since (setEffort); "" is Claude Code's own effort string + // loginVersion is the credential the run's account had when it + // started (provider.ClaudeLoginVersion), which a refusal it reports is + // kept for loginVersion string // told is the conversation as the client had it in its last request @@ -856,6 +859,9 @@ func (r *subscriptionRun) readOutput(rd io.Reader) { text += ": " + strings.Join(envelope.Errors, "; ") } } + // Anthropic refused the account's sign-in: kept on it, so + // it isn't run again on that one (provider/claude_auth.go) — + // not when Claude Code's own run went on as another account if fields := strings.Split(r.owner, "\x00"); len(fields) > 1 && !(len(fields) > 2 && fields[2] == ownHome && provider.ClaudeCodeMovedOff(fields[1])) { provider.NoteClaudeSignInFailure(fields[1], r.loginVersion, text) diff --git a/internal/gateway/fallback.go b/internal/gateway/fallback.go index ddedd39f1..5d111d0a1 100644 --- a/internal/gateway/fallback.go +++ b/internal/gateway/fallback.go @@ -133,7 +133,7 @@ func perKeyBarred(p provider.Provider, model string, from provider.Protocol) (ou var all []candidate // the account the agent is signed in to, unless the user paused // it for the others on (#263) - if p.Account.SignInError() == "" && (len(also) == 0 || !p.OwnPaused()) { + if len(also) == 0 || !p.OwnPaused() { all = append(all, candidate{p: p, model: model, rest: p.ID}) } for i, q := range also { diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index 50be415c6..8cc040fbf 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -1650,6 +1650,10 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request, from provider.Pro continue } if !last && hw.failed() && failure(hw.code(), hw.errBody()) == failAuth { + // the account's sign-in is gone, refused by Anthropic: no rest + // brings it back, so none is told; it is passed over until it + // is signed in again (provider/claude_auth.go), and the next + // one is asked if other == nil { other = &Try{Status: call.Status, Error: call.Error} } diff --git a/internal/provider/claude_auth.go b/internal/provider/claude_auth.go index 20728dd49..a3d0dd067 100644 --- a/internal/provider/claude_auth.go +++ b/internal/provider/claude_auth.go @@ -1,23 +1,37 @@ package provider +// A Claude sign-in Anthropic refused. Claude Code refreshes the sign-ins it +// runs on itself; when Anthropic refuses a refresh (invalid_grant), Claude +// Code empties the tokens where it keeps them (Claude Code 2.1.x), and each +// run on that sign-in fails with "OAuth session expired and could not be +// refreshed" until the account is signed in again. Waiting doesn't mend it, +// so magpie doesn't rest the account and try it again: it keeps the refusal +// on the saved login, for the credential it was made on (Refused), and +// passes over the login while it still has that credential. A credential +// changed since — Claude Code refreshed it after all, or the account was +// signed in again — is not the one refused. + import ( "crypto/sha256" - "fmt" + "encoding/hex" "strings" ) +// claudeAuthLapse is why a saved Claude account whose credential Anthropic +// refused can't be used. const claudeAuthLapse = "Claude Code could not authenticate this account; sign in again in magpie" -// ClaudeSignInRequired recognizes a CLI refusal that needs a new login, -// not a network failure or another process temporarily holding the refresh lock. +// ClaudeSignInRequired says message is Claude Code's, or magpie's, word that +// a Claude sign-in is gone and has to be made again: not a failure of the +// moment, as a network error or another Claude Code holding the refresh is +// ("Failed to refresh OAuth token: another Claude Code process is +// refreshing it"). func ClaudeSignInRequired(message string) bool { for _, text := range []string{ - "OAuth session expired and could not be refreshed", - "OAuth access token has been revoked", - "OAuth token revoked", - claudeAuthLapse, - claudeLogoutLapse, - legacyClaudeLogoutLapse, + "OAuth session expired and could not be refreshed", // Claude Code: its refresh refused + "OAuth token revoked", // Claude Code: Anthropic revoked it + "OAuth access token has been revoked", // Anthropic's own 401 + claudeAuthLapse, claudeLogoutLapse, claudeGoneLapse, } { if strings.Contains(message, text) { return true @@ -26,16 +40,30 @@ func ClaudeSignInRequired(message string) bool { return false } +// version tells a credential from the one it is refreshed to: a refresh +// replaces both tokens. +func (c claudeCredentials) version() string { + sum := sha256.Sum256([]byte(c.OAuth.AccessToken + "\x00" + c.OAuth.RefreshToken)) + return hex.EncodeToString(sum[:]) +} + +// cleared says c is a sign-in Claude Code emptied, as it does one whose +// refresh Anthropic refused: it reads an empty refresh token as no sign-in. +func (c claudeCredentials) cleared() bool { + o, _ := c.raw["claudeAiOauth"].(map[string]any) + rt, ok := o["refreshToken"].(string) + return ok && rt == "" && c.OAuth.AccessToken == "" +} + func claudeLoginVersion(l savedLogin) string { - c, ok := parseClaudeCredentials(l.Auth) - if !ok { - return "" + if c, ok := parseClaudeCredentials(l.Auth); ok { + return c.version() } - return fmt.Sprintf("%x", sha256.Sum256([]byte(c.OAuth.AccessToken+"\x00"+c.OAuth.RefreshToken))) + return "" } -// ClaudeLoginVersion identifies the saved login a CLI run starts with. -// An error from an older run must not invalidate a newly authorized login. +// ClaudeLoginVersion is the credential the saved Claude account user has +// now, which a run started on it goes by (NoteClaudeSignInFailure). func ClaudeLoginVersion(user string) string { loginsMu.Lock() defer loginsMu.Unlock() @@ -47,35 +75,51 @@ func ClaudeLoginVersion(user string) string { return "" } -// NoteClaudeSignInFailure records a terminal authentication refusal for -// this login only. The original CLI error remains in the request trace. +// refuseClaudeLogin keeps on l that Anthropic refused its credential +// version, and says whether that changed l. +func refuseClaudeLogin(l *savedLogin, version string) bool { + if version == "" || l.Refused == version && l.Lapsed == claudeAuthLapse { + return false + } + l.Lapsed, l.Refused = claudeAuthLapse, version + return true +} + +// NoteClaudeSignInFailure keeps a run's refusal on the saved Claude account +// user, when the run started on the credential the account still has +// (version): one started on an older credential says nothing of this one. +// The run's own error stays in the request's trace. func NoteClaudeSignInFailure(user, version, message string) { - if version == "" || !ClaudeSignInRequired(message) { + if !ClaudeSignInRequired(message) { return } loginsMu.Lock() defer loginsMu.Unlock() ls := readLogins() for i := range ls { - if ls[i].Agent == "claude" && strings.EqualFold(ls[i].User, user) && - claudeLoginVersion(ls[i]) == version && ls[i].Lapsed == "" { - ls[i].Lapsed = claudeAuthLapse - _ = writeLogins(ls) + if ls[i].Agent == "claude" && strings.EqualFold(ls[i].User, user) && claudeLoginVersion(ls[i]) == version { + if refuseClaudeLogin(&ls[i], version) { + _ = writeLogins(ls) + } return } } } -// SignInError is a saved Claude account's terminal login state. -func (a *Account) SignInError() string { - if a == nil || a.Agent != "claude" { +// claudeOwnRefused is why the account Claude Code itself is signed in to, +// user, can't be used: Anthropic refused the credential Claude Code holds +// for it now; "" when it didn't. +func claudeOwnRefused(user string) string { + c, _, ok := claudeCredential() + if !ok { return "" } + v := c.version() loginsMu.Lock() defer loginsMu.Unlock() for _, l := range readLogins() { - if l.Agent == "claude" && strings.EqualFold(l.User, a.User) && l.Lapsed != "" { - return claudeSignedOut(l) + if l.Agent == "claude" && strings.EqualFold(l.User, user) && l.Refused == v && l.Lapsed != "" { + return l.Lapsed } } return "" diff --git a/internal/provider/claude_auth_test.go b/internal/provider/claude_auth_test.go index 451d7218f..e7e6583d6 100644 --- a/internal/provider/claude_auth_test.go +++ b/internal/provider/claude_auth_test.go @@ -34,47 +34,89 @@ func claudeAuthFixture(t *testing.T, lapse string) (Provider, savedLogin) { return p, side } -func TestClaudeLapsedLoginIsNotRoutedOrRestored(t *testing.T) { - p, side := claudeAuthFixture(t, legacyClaudeLogoutLapse) - if also := p.AlsoOn(); len(also) != 0 { - t.Fatalf("lapsed account still routed: %+v", also) +// sideToken is the saved account's turn at a request: the directory Claude +// Code runs on it in, or why it can't. +func sideToken(t *testing.T, p Provider, user string) (string, error) { + t.Helper() + for _, q := range p.AlsoOn() { + if q.Account.User == user { + dir, _, err := q.Account.Token(context.Background()) + return dir, err + } } - if _, err := claudeSavedDir(side.User); err == nil || !strings.Contains(err.Error(), "sign in again") { - t.Fatalf("lapsed login restored: %v", err) + t.Fatalf("%s is not among the accounts on", user) + return "", nil +} + +func claudeLapseOf(user string) string { + for _, l := range Logins("claude") { + if strings.EqualFold(l.User, user) { + return l.Lapsed + } + } + return "" +} + +func TestClaudeLegacyLapseIsNotRunOrBlamedOnLogout(t *testing.T) { + p, side := claudeAuthFixture(t, legacyClaudeLogoutLapse) + if _, err := sideToken(t, p, side.User); err == nil || !strings.Contains(err.Error(), "sign in again") { + t.Fatalf("lapsed login run: %v", err) } if _, err := os.Stat(claudeAccountDir(side.User)); !os.IsNotExist(err) { - t.Fatalf("a directory was created for the lapsed login: %v", err) + t.Fatalf("a directory was made for the lapsed login: %v", err) } - for _, l := range Logins("claude") { - if l.User == side.User && (l.Lapsed != claudeLogoutLapse || strings.Contains(l.Lapsed, "/logout")) { - t.Fatalf("absence blamed on logout: %+v", l) - } + if got := claudeLapseOf(side.User); got != claudeLogoutLapse { + t.Fatalf("lapse shown as %q", got) } } -func TestClaudeAuthFailureRequiresNewLoginAndIgnoresOldRuns(t *testing.T) { +// A refusal holds for the credential it was made on: Claude Code refreshing +// the account's sign-in in its directory afterwards, or a run started on +// the refused credential reporting late, leaves the account usable. +func TestClaudeRefusalHoldsForItsCredentialOnly(t *testing.T) { p, side := claudeAuthFixture(t, "") - oldVersion := ClaudeLoginVersion(side.User) - NoteClaudeSignInFailure(side.User, oldVersion, "Failed to authenticate: OAuth session expired and could not be refreshed") - if len(p.AlsoOn()) != 0 { - t.Fatal("authentication failure was not removed from routing") + refused := ClaudeLoginVersion(side.User) + NoteClaudeSignInFailure(side.User, refused, "Failed to authenticate: OAuth session expired and could not be refreshed") + if _, err := sideToken(t, p, side.User); err == nil || !strings.Contains(err.Error(), "sign in again") { + t.Fatalf("refused login run: %v", err) } if err := SwitchLogin("claude", side.User); err == nil { - t.Fatal("switching restored a rejected login") + t.Fatal("Claude Code signed in to a refused login") + } + writeFile(t, filepath.Join(claudeAccountDir(side.User), ".credentials.json"), map[string]any{"claudeAiOauth": map[string]any{ + "accessToken": "new-access", "refreshToken": "new-refresh", + "expiresAt": time.Now().Add(8 * time.Hour).UnixMilli(), "subscriptionType": "max", + }}) + if _, err := sideToken(t, p, side.User); err != nil { + t.Fatalf("the sign-in Claude Code refreshed to is not used: %v", err) } - c, _ := parseClaudeCredentials(side.Auth) - c.OAuth.AccessToken, c.OAuth.RefreshToken = "new-access", "new-refresh" - side.Auth, _ = c.marshal() - if _, err := addLogin(side); err != nil { + NoteClaudeSignInFailure(side.User, refused, "Failed to authenticate: OAuth token revoked.") + if _, err := sideToken(t, p, side.User); err != nil || claudeLapseOf(side.User) != "" { + t.Fatalf("an old run's refusal holds for the new sign-in: %v", err) + } +} + +// Claude Code empties a sign-in Anthropic refused to refresh: the copy +// magpie kept of it is the refused one, never put back for Claude Code. +func TestClaudeEmptiedSignInIsNotPutBack(t *testing.T) { + p, side := claudeAuthFixture(t, "") + dir, err := sideToken(t, p, side.User) + if err != nil { t.Fatal(err) } - NoteClaudeSignInFailure(side.User, oldVersion, "OAuth token revoked") - also := p.AlsoOn() - if len(also) != 1 { - t.Fatalf("an old run invalidated the new login: %+v", also) + creds := filepath.Join(dir, ".credentials.json") + writeFile(t, creds, map[string]any{"claudeAiOauth": map[string]any{"accessToken": "", "refreshToken": "", "expiresAt": 0}}) + if err := SwitchLogin("claude", side.User); err == nil { + t.Fatal("Claude Code signed in to the refused copy") + } + if _, err := sideToken(t, p, side.User); err == nil { + t.Fatal("ran on a sign-in Claude Code emptied") } - if _, _, err := also[0].Account.Token(context.Background()); err != nil { - t.Fatalf("new login could not be used: %v", err) + if b, _ := os.ReadFile(creds); strings.Contains(string(b), "old-refresh") { + t.Fatal("the refused copy was put back") + } + if claudeLapseOf(side.User) != claudeAuthLapse { + t.Fatalf("lapse: %q", claudeLapseOf(side.User)) } } @@ -82,33 +124,70 @@ func TestClaudeClearedKeychainDoesNotFallBackToStaleFile(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("a shell script stands in for the keychain") } - _, side := claudeAuthFixture(t, "") - dir, err := claudeSavedDir(side.User) + p, side := claudeAuthFixture(t, "") + dir, err := sideToken(t, p, side.User) if err != nil { t.Fatal(err) } bin := t.TempDir() - tombstone := filepath.Join(bin, "cleared.json") - writeFile(t, tombstone, map[string]any{"claudeAiOauth": map[string]any{ + cleared := filepath.Join(bin, "cleared.json") + writeFile(t, cleared, map[string]any{"claudeAiOauth": map[string]any{ "accessToken": "", "refreshToken": "", "expiresAt": 0, "subscriptionType": "max", }}) - script := "#!/bin/sh\ncat '" + tombstone + "'\n" - if err := os.WriteFile(filepath.Join(bin, "security"), []byte(script), 0o755); err != nil { + if err := os.WriteFile(filepath.Join(bin, "security"), []byte("#!/bin/sh\ncat '"+cleared+"'\n"), 0o755); err != nil { t.Fatal(err) } t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) claudeKeychain = true - if _, err := claudeSavedDir(side.User); err == nil { - t.Fatal("cleared keychain fell back to the old credentials file") + if _, err := sideToken(t, p, side.User); err == nil { + t.Fatal("cleared keychain item passed over for the file beside it") } - b, _ := os.ReadFile(filepath.Join(dir, ".credentials.json")) - if !strings.Contains(string(b), "old-refresh") { - t.Fatal("the stale file was rewritten rather than refusing it") + if b, _ := os.ReadFile(filepath.Join(dir, ".credentials.json")); !strings.Contains(string(b), "old-refresh") { + t.Fatal("the file was rewritten") } - for _, l := range Logins("claude") { - if l.User == side.User && l.Lapsed == "" { - t.Fatal("cleared saved login was not marked as needing sign-in") - } +} + +// The account Claude Code itself is signed in to: read again with the same +// sign-in after a moment it couldn't be read, it is not taken for gone; +// refused, it stays so until Claude Code holds another credential. +func TestClaudeOwnLoginLapsesOnlyForWhatHappened(t *testing.T) { + p, _ := claudeAuthFixture(t, "") + cred := filepath.Join(os.Getenv("HOME"), ".claude", ".credentials.json") + saved, err := os.ReadFile(cred) + if err != nil { + t.Fatal(err) + } + look := func() { + forgetClaudeCredential() + loginsMu.Lock() + loginsSeenAt = time.Time{} + loginsMu.Unlock() + rememberLogins(true) + } + look() + os.Remove(cred) + look() + if err := os.WriteFile(cred, saved, 0o600); err != nil { + t.Fatal(err) + } + look() + if got := claudeLapseOf("own@example.com"); got != "" { + t.Fatalf("read again unchanged, still lapsed: %q", got) + } + + NoteClaudeSignInFailure("own@example.com", ClaudeLoginVersion("own@example.com"), + "Failed to authenticate: OAuth session expired and could not be refreshed") + look() + if _, _, err := p.Account.Token(context.Background()); err == nil || claudeLapseOf("own@example.com") == "" { + t.Fatalf("refused own login still run: %v", err) + } + writeFile(t, cred, map[string]any{"claudeAiOauth": map[string]any{ + "accessToken": "signed-in-again", "refreshToken": "signed-in-again-refresh", + "expiresAt": time.Now().Add(time.Hour).UnixMilli(), "subscriptionType": "max", + }}) + look() + if _, _, err := p.Account.Token(context.Background()); err != nil || claudeLapseOf("own@example.com") != "" { + t.Fatalf("signed in again, still refused: %v", err) } } @@ -118,6 +197,7 @@ func TestClaudeSignInRequiredDoesNotBlameTemporaryFailures(t *testing.T) { want bool }{ {"Failed to authenticate: OAuth session expired and could not be refreshed", true}, + {"Failed to authenticate: OAuth token revoked. Please log in again or contact your administrator.", true}, {"OAuth access token has been revoked.", true}, {"Failed to refresh OAuth token: another Claude Code process is refreshing it or exited mid-refresh", false}, {"OAuth token refresh failed (HTTP 503)", false}, @@ -137,12 +217,15 @@ func TestClaudeProbeReportsTheAccountAndMarksItsFailedLogin(t *testing.T) { claudeCLIProbe = func(context.Context, string, string) error { return errors.New("Failed to authenticate: OAuth session expired and could not be refreshed") } - saved := p.AlsoOn()[0] + var saved Provider + for _, q := range p.AlsoOn() { + saved = q + } result := saved.testClaude(context.Background(), "claude-sonnet-5") if result.OK || result.Account != side.User || !strings.Contains(result.Error, "OAuth session expired") { t.Fatalf("failed test lost the account or original error: %+v", result) } - if len(p.AlsoOn()) != 0 { - t.Fatal("the login that failed its connection test is still routed") + if _, err := sideToken(t, p, side.User); err == nil { + t.Fatal("the login that failed its test is still run") } } diff --git a/internal/provider/claude_dirs.go b/internal/provider/claude_dirs.go index 9abff3a42..c9c7c4ddc 100644 --- a/internal/provider/claude_dirs.go +++ b/internal/provider/claude_dirs.go @@ -50,8 +50,8 @@ func readClaudeDir(dir string) (claudeCredentials, bool) { if err == nil { b, _ := keychainText(bytes.TrimSpace(out)) if c, ok := parseClaudeCredentials(b); c.raw != nil { - // An existing, cleared keychain record is authoritative: - // a file left beside it must not resurrect the rejected login. + // the item there is the sign-in, emptied or not: Claude + // Code reads no file beside it return c, ok } } @@ -81,7 +81,8 @@ func forgetClaudeDir(user string) { // claudeSavedDir is the config directory to run Claude Code in for the // saved account user: its sign-in is put there from logins.json when there -// is none yet, and what Claude Code has made of it since is read back. +// is none yet, and what Claude Code has made of it since is read back. An +// account that can't be used (claudeSignedOut) gets none. func claudeSavedDir(user string) (string, error) { savedTokenMu.Lock() defer savedTokenMu.Unlock() @@ -97,28 +98,23 @@ func claudeSavedDir(user string) (string, error) { if i < 0 { return "", fmt.Errorf("no saved claude account %q", user) } - if ls[i].Lapsed != "" { - return "", errors.New(claudeSignedOut(ls[i])) - } dir := claudeAccountDir(user) - c, ok := readClaudeDir(dir) - if !ok && c.raw != nil { - ls[i].Lapsed = claudeAuthLapse + has, changed, err := syncClaudeDir(&ls[i]) + if err != nil { + return "", err + } + if changed { if err := writeLogins(ls); err != nil { return "", err } - return "", errors.New(claudeAuthLapse) } - if ok { - if changed, err := takeClaudeDir(&ls[i], c); err != nil || !changed { - return dir, err - } - return dir, writeLogins(ls) + if why := claudeSignedOut(ls[i]); why != "" { + return "", errors.New(why) } - c, ok = parseClaudeCredentials(ls[i].Auth) - if !ok { - return "", errors.New("the saved Claude sign-in of " + user + " is unreadable") + if has { + return dir, nil } + c, _ := parseClaudeCredentials(ls[i].Auth) b, err := c.marshal() if err != nil { return "", err @@ -141,6 +137,23 @@ func claudeSavedDir(user string) (string, error) { return dir, nil } +// syncClaudeDir brings the saved Claude login l up to what Claude Code +// keeps in the account's config directory: the sign-in it refreshed to +// there, or, when it emptied that sign-in, Anthropic's refusal of the one +// l has. has says the directory holds a sign-in to run on; changed, that l +// changed. +func syncClaudeDir(l *savedLogin) (has, changed bool, err error) { + c, ok := readClaudeDir(claudeAccountDir(l.User)) + switch { + case ok: + changed, err = takeClaudeDir(l, c) + return true, changed, err + case c.cleared(): + return false, refuseClaudeLogin(l, claudeLoginVersion(*l)), nil + } + return false, false, nil +} + // takeClaudeDir puts the sign-in Claude Code keeps in an account's config // directory into its saved login, and says whether that changed it. func takeClaudeDir(l *savedLogin, c claudeCredentials) (bool, error) { @@ -157,7 +170,7 @@ func takeClaudeDir(l *savedLogin, c claudeCredentials) (bool, error) { l.Plan = c.OAuth.SubscriptionType } l.Renewed = time.Now().UTC().Truncate(time.Second) - l.Lapsed = "" + l.Lapsed, l.Refused = "", "" return true, nil } @@ -167,20 +180,20 @@ func takeClaudeDir(l *savedLogin, c claudeCredentials) (bool, error) { // magpie keeps the other accounts' sign-ins itself, each its own, so a // logout signs none of them out of magpie: the stand-in runs Claude Code // in a config directory of its own, as the others on do. Never the one -// Claude Code was signed in to, whose sign-in its /logout revoked +// Claude Code was signed in to, whose sign-in went with it // (claudeLoggedOut, or still Held before rememberLogins next looks: it is -// asked only while Claude Code is signed out); of the others, one with a -// saved sign-in, on before off, then the one seen last; one whose saved -// sign-in is gone only when no other has one; "" when there is none. +// asked only while Claude Code is signed out); of the others, one that can +// be used, on before off, then the one seen last; one that can't — its +// sign-in gone, or refused — only when no other can; "" when there is none. func claudeStandIn(ls []savedLogin) string { user, best := "", -1 var seen time.Time for _, l := range ls { - if l.Agent != "claude" || l.Held || l.Lapsed != "" { + if l.Agent != "claude" || l.Held || l.Lapsed != "" && l.Refused == "" { continue } rank := 0 - if _, ok := parseClaudeCredentials(l.Auth); ok { + if claudeSignedOut(l) == "" { rank = 2 if l.On { rank++ @@ -194,16 +207,26 @@ func claudeStandIn(ls []savedLogin) string { } // claudeLogoutLapse is why the account Claude Code was signed in to can't -// be used once Claude Code logged out. +// be used once Claude Code let go of it: logged out, which revokes the +// sign-in it held, or emptied that sign-in once Anthropic refused it. const claudeLogoutLapse = "Claude Code is no longer signed in; sign in again in magpie" +// legacyClaudeLogoutLapse is claudeLogoutLapse as magpie wrote it before, +// which took every such lapse for a /logout. const legacyClaudeLogoutLapse = "Claude Code's /logout signed it out (it revokes the sign-in it holds); sign in again" -// claudeLoggedOut marks the account Claude Code held, now that it is -// signed out, as lapsed. The absence of a login does not tell whether the -// user logged out or Claude Code cleared a rejected refresh token. -// The accounts magpie keeps in config directories of their own are -// sign-ins of their own and stay. It says whether ls changed. +// claudeGoneLapse is why a saved Claude account with no sign-in kept can't +// be used. +const claudeGoneLapse = "its sign-in is gone; sign in again" + +// claudeLoggedOut marks the account Claude Code held, now that it holds no +// sign-in, as lapsed: magpie's copy of that account is the same sign-in, +// gone too — revoked by a /logout (POST /revoke, Claude Code +// 2.1.x's performLogout), or refused by Anthropic, as the sign-in Claude +// Code emptied was. Which it was can't be told from here. Signed in to it +// again, Claude Code holds it once more (rememberLogins). The accounts +// magpie keeps in config directories of their own are sign-ins of their own +// and stay. It says whether ls changed. func claudeLoggedOut(ls []savedLogin) bool { changed := false for i := range ls { @@ -211,17 +234,19 @@ func claudeLoggedOut(ls []savedLogin) bool { continue } ls[i].Held, changed = false, true - if _, ok := parseClaudeCredentials(ls[i].Auth); ok && ls[i].Lapsed == "" { - ls[i].Lapsed = claudeLogoutLapse + if claudeSignedOut(ls[i]) == "" { + ls[i].Lapsed, ls[i].Refused = claudeLogoutLapse, "" } } return changed } -// claudeSignedOut is why a saved Claude account can't be used: its saved -// sign-in is gone, and it has to be signed in again; "" when it has one. +// claudeSignedOut is why a saved Claude account can't be used, "" when it +// can: its saved sign-in is gone, Claude Code let go of it +// (claudeLoggedOut), or Anthropic refused the credential it has now +// (claude_auth.go) — a refusal of an earlier one says nothing of it. func claudeSignedOut(l savedLogin) string { - if l.Lapsed != "" { + if l.Lapsed != "" && (l.Refused == "" || l.Refused == claudeLoginVersion(l)) { if l.Lapsed == legacyClaudeLogoutLapse { return claudeLogoutLapse } @@ -230,7 +255,7 @@ func claudeSignedOut(l savedLogin) string { if _, ok := parseClaudeCredentials(l.Auth); ok { return "" } - return "its sign-in is gone; sign in again" + return claudeGoneLapse } // claudeStandInAccount is the Claude Code provider while Claude Code is diff --git a/internal/provider/logins.go b/internal/provider/logins.go index 58c24903d..eda57c729 100644 --- a/internal/provider/logins.go +++ b/internal/provider/logins.go @@ -83,6 +83,10 @@ type savedLogin struct { // Lapsed why the vendor last refused to (logins_on.go, keepalive.go). Renewed time.Time `json:"renewed,omitzero"` Lapsed string `json:"lapsed,omitempty"` + // Refused is the Claude credential Anthropic refused (its version, in + // claude_auth.go): Lapsed holds while the account has that one, and + // says nothing of the one it is refreshed or signed in to next. + Refused string `json:"refused,omitempty"` // Hidden is the agent's own sign-in removed in magpie, with the mark // of the sign-in it was (side_logins.go): it is listed and tried no // more until the agent signs in anew. The agent's files stay as they are. @@ -179,9 +183,10 @@ func writePrivate(path string, b []byte) error { func upsertLogin(ls []savedLogin, l savedLogin) []savedLogin { for i := range ls { if sameLogin(ls[i], l) { - if l.Agent == "claude" && l.Lapsed == "" && ls[i].Lapsed != "" && - claudeLoginVersion(l) == claudeLoginVersion(ls[i]) { - l.Lapsed = ls[i].Lapsed + // a refused Claude credential stays refused while it is the + // one the account has + if l.Agent == "claude" && l.Lapsed == "" && ls[i].Refused != "" && ls[i].Refused == claudeLoginVersion(l) { + l.Lapsed, l.Refused = ls[i].Lapsed, ls[i].Refused } l.On = l.On || ls[i].On l.Paused = l.Paused || ls[i].Paused @@ -772,15 +777,18 @@ func switchSavedLogin(agent, user string) (from string, _ error) { return "", fmt.Errorf("no saved %s account %q", agent, user) } if agent == "claude" { - if target.Lapsed != "" { - return "", errors.New(claudeSignedOut(*target)) - } // as Claude Code keeps it, if it has run on the account beside the - // one it is signed in to - if c, ok := readClaudeDir(claudeAccountDir(target.User)); ok { - if _, err := takeClaudeDir(target, c); err != nil { - return "", err + // one it is signed in to; never one that can't be used, which + // Claude Code would only be refused on + _, changed, err := syncClaudeDir(target) + if err != nil { + return "", err + } + if why := claudeSignedOut(*target); why != "" { + if changed { + _ = writeLogins(ls) } + return "", fmt.Errorf("%s: %s", target.User, why) } } want := *target diff --git a/internal/provider/logins_on.go b/internal/provider/logins_on.go index b3f1c08c2..efe86356f 100644 --- a/internal/provider/logins_on.go +++ b/internal/provider/logins_on.go @@ -170,7 +170,7 @@ func (p Provider) AlsoOn() []Provider { } var out []Provider for _, l := range Logins(p.Account.Agent) { - if l.Active || l.first || !l.On || l.Lapsed != "" { + if l.Active || l.first || !l.On { continue } agent, user := l.Agent, l.User @@ -197,12 +197,19 @@ func (p Provider) AlsoOn() []Provider { // Token is the access token of a saved account in use beside the agent's // own — for a Claude account, the config directory Claude Code runs on it -// in; ok is false for the agent's own, which the agent signs itself. +// in; ok is false for the agent's own, which the agent signs itself. A +// Claude account that can't be used, Anthropic having refused its sign-in, +// is an error, whichever it is (claude_auth.go). func (a *Account) Token(ctx context.Context) (tok string, ok bool, err error) { - if msg := a.SignInError(); msg != "" { - return "", false, errors.New(msg) + if a == nil { + return "", false, nil } - if a == nil || a.token == nil { + if a.token == nil { + if a.Agent == "claude" { + if why := claudeOwnRefused(a.User); why != "" { + return "", false, errors.New(why) + } + } return "", false, nil } tok, err = a.token(ctx) From eceedde47fe8654a3e3ae62e5ee0c4702d2afe98 Mon Sep 17 00:00:00 2001 From: Haogao Gu Date: Sat, 3 Oct 2026 16:36:29 -0400 Subject: [PATCH 3/3] Limit sign-in lapses to Claude accounts Address the review of #716: - A refused sign-in is failAuth only for a Claude account (failureOf); another vendor with the same words rests as before. - The account list's sign-in-required row is Claude's only, and the account Claude Code is signed in to offers no Remove. - Add the missing zh string for "its sign-in is gone; sign in again". - readOutput checks the error before reading Claude Code's sign-in, and the owner is parsed in one place (ownerAccount). TestSignInWordsFromAnotherVendorRest and the new browser case in claude-auth-status.test.cjs fail on the previous commit and pass. --- internal/gateway/claude_auth_test.go | 27 +++++++++++ internal/gateway/claude_subscription.go | 27 +++++++---- internal/gateway/gateway.go | 8 ++-- internal/gateway/routing.go | 16 +++++-- internal/gui/assets/app.js | 24 ++++++---- internal/gui/assets/i18n.js | 1 + .../gui/tests/claude-auth-status.test.cjs | 48 ++++++++++++++++++- 7 files changed, 125 insertions(+), 26 deletions(-) diff --git a/internal/gateway/claude_auth_test.go b/internal/gateway/claude_auth_test.go index 666866d96..ec6f626b7 100644 --- a/internal/gateway/claude_auth_test.go +++ b/internal/gateway/claude_auth_test.go @@ -3,6 +3,8 @@ package gateway import ( "context" "encoding/json" + "io" + "net/http" "net/http/httptest" "os" "path/filepath" @@ -104,3 +106,28 @@ func TestClaudeAllRefusedSaysSignInAgain(t *testing.T) { t.Fatalf("not told to sign in again: %d %s", rec.Code, rec.Body.String()) } } + +// Another vendor saying a Claude sign-in's words (an OpenCode plugin on +// Anthropic's OAuth: "OAuth access token has been revoked") isn't a Claude +// account Anthropic refused: it rests as any failure does, and isn't told +// as needing a sign-in that magpie would never try again (#716 review). +func TestSignInWordsFromAnotherVendorRest(t *testing.T) { + fresh(t) + revoked := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + io.WriteString(w, `{"error":{"message":"OAuth access token has been revoked"}}`) + }) + serveOn(t, "a", "ka", []string{"m"}, revoked) + serveOn(t, "b", "kb", []string{"m"}, &keyed{}) + if err := provider.SaveGroup(provider.Group{Name: "Two", Members: []string{"a/m", "b/m"}, Routing: provider.Ordered}); err != nil { + t.Fatal(err) + } + s := New() + if code, body := postAs(t, s, "", `{"model":"group/two","messages":[{"role":"user","content":"hi"}]}`); code != 200 || !strings.Contains(body, "from kb") { + t.Fatalf("%d %s", code, body) + } + if r := lastRoute(s); len(r.Tries) != 2 || r.Tries[0].Fail == failAuth || r.Tries[0].Rest == nil { + t.Fatalf("not rested as before: %+v", r.Tries) + } +} diff --git a/internal/gateway/claude_subscription.go b/internal/gateway/claude_subscription.go index b53a6c929..a10306f34 100644 --- a/internal/gateway/claude_subscription.go +++ b/internal/gateway/claude_subscription.go @@ -330,8 +330,8 @@ func (b *subscriptionBridge) start(ctx context.Context, req *Request, model, con } run := &subscriptionRun{bridge: b, token: token, model: model, cmd: cmd, tmp: tmp, schema: len(req.Schema) > 0, pending: map[string]chan mcpToolResult{}, stdin: stdin, owner: owner, effort: req.Effort} - if fields := strings.Split(owner, "\x00"); len(fields) > 1 { - run.loginVersion = provider.ClaudeLoginVersion(fields[1]) + if user, _ := ownerAccount(owner); user != "" { + run.loginVersion = provider.ClaudeLoginVersion(user) } // A caller may abandon a turn after receiving tool_use. Do not leave the // parked Claude process and MCP request alive forever. @@ -839,8 +839,8 @@ func (r *subscriptionRun) readOutput(rd io.Reader) { // a run in Claude Code's own home is on whatever account // Claude Code is signed in to by now: switched off the // owner's, what it says is another's (nil_1024) - if f := strings.Split(r.owner, "\x00"); len(f) > 1 && !(len(f) > 2 && f[2] == ownHome && provider.ClaudeCodeMovedOff(f[1])) { - provider.NoteClaudeLimits(f[1], claudeLimits(envelope.RateLimitInfo)) + if user, own := ownerAccount(r.owner); user != "" && !(own && provider.ClaudeCodeMovedOff(user)) { + provider.NoteClaudeLimits(user, claudeLimits(envelope.RateLimitInfo)) } continue } @@ -861,10 +861,11 @@ func (r *subscriptionRun) readOutput(rd io.Reader) { } // Anthropic refused the account's sign-in: kept on it, so // it isn't run again on that one (provider/claude_auth.go) — - // not when Claude Code's own run went on as another account - if fields := strings.Split(r.owner, "\x00"); len(fields) > 1 && - !(len(fields) > 2 && fields[2] == ownHome && provider.ClaudeCodeMovedOff(fields[1])) { - provider.NoteClaudeSignInFailure(fields[1], r.loginVersion, text) + // not when Claude Code's own run went on as another account, + // which is read only for such an error + if user, own := ownerAccount(r.owner); user != "" && provider.ClaudeSignInRequired(text) && + !(own && provider.ClaudeCodeMovedOff(user)) { + provider.NoteClaudeSignInFailure(user, r.loginVersion, text) } r.emit(Event{Kind: KError, Text: text, Status: envelope.APIErrorStatus, Code: errKind, RequestID: reqID}) r.endSegment() @@ -1498,6 +1499,16 @@ func (b *subscriptionBridge) removeRun(run *subscriptionRun) { // own home. const ownHome = "own" +// ownerAccount is the account a run's owner names, "" when it names none, +// and whether it is Claude Code's own sign-in (ownHome). +func ownerAccount(owner string) (user string, own bool) { + f := strings.Split(owner, "\x00") + if len(f) < 2 { + return "", false + } + return f[1], len(f) > 2 && f[2] == ownHome +} + func (s *Server) serveClaudeSubscription(w http.ResponseWriter, r *http.Request, from provider.Protocol, p provider.Provider, model string, body []byte, usage *Usage) (int, string) { start := func(ctx context.Context, req *Request) (*subscriptionRun, <-chan Event, error) { ctx = p.Via(ctx) // the account's own proxy, its CLI run's too diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index 8cc040fbf..86174ec13 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -1649,7 +1649,7 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request, from provider.Pro skipped = append(skipped, c.label()+": "+call.Error) continue } - if !last && hw.failed() && failure(hw.code(), hw.errBody()) == failAuth { + if !last && hw.failed() && failureOf(c, hw.code(), hw.errBody()) == failAuth { // the account's sign-in is gone, refused by Anthropic: no rest // brings it back, so none is told; it is passed over until it // is signed in again (provider/claude_auth.go), and the next @@ -1666,7 +1666,7 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request, from provider.Pro // the others left are out of their allowance (Discord, waroy: a // Codex account run out, Grok busy a moment): this one is the // last that may answer, and is tried again as the last is - try.Fail, try.Again = failure(hw.code(), hw.errBody()), wait.Milliseconds() + try.Fail, try.Again = failureOf(c, hw.code(), hw.errBody()), wait.Milliseconds() s.trace.update(tr, func(t *Route) { t.Tries[len(t.Tries)-1] = try }) skipped = append(skipped, c.label()+": "+call.Error) again++ @@ -1691,7 +1691,7 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request, from provider.Pro } if wait, ok := passing(hw.code(), hw.header, hw.errBody(), again); ok && hw.failed() { // nobody else is left: the same one again, after a moment - try.Fail, try.Again = failure(hw.code(), hw.errBody()), wait.Milliseconds() + try.Fail, try.Again = failureOf(c, hw.code(), hw.errBody()), wait.Milliseconds() s.trace.update(tr, func(t *Route) { t.Tries[len(t.Tries)-1] = try }) skipped = append(skipped, c.label()+": "+call.Error) again++ @@ -1760,7 +1760,7 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request, from provider.Pro } else if hw.refused { try.Fail = failRefused } else { - try.Fail = failure(call.Status, []byte(call.Error)) + try.Fail = failureOf(c, call.Status, []byte(call.Error)) if try.Fail == failVerify && !held { // the last one left rests too, for the app to show and the // next requests to be held diff --git a/internal/gateway/routing.go b/internal/gateway/routing.go index bf05eaa9e..c9a8f0656 100644 --- a/internal/gateway/routing.go +++ b/internal/gateway/routing.go @@ -169,9 +169,6 @@ var proxyDown = regexp.MustCompile(`proxyconnect |socks connect `) // failure says why a reply failed. func failure(status int, body []byte) string { - if status >= 400 && provider.ClaudeSignInRequired(string(body)) { - return failAuth - } if status == http.StatusBadGateway && proxyDown.Match(body) { return failProxy } @@ -195,6 +192,17 @@ func failure(status int, body []byte) string { return failOther } +// failureOf says why c's reply failed: a Claude account's sign-in that +// Anthropic refused is told as one (provider/claude_auth.go), passed over +// till it is signed in again; another vendor saying the same words fails +// as failure says, and rests as before. +func failureOf(c candidate, status int, body []byte) string { + if status >= 400 && c.p.Account != nil && c.p.Account.Agent == "claude" && provider.ClaudeSignInRequired(string(body)) { + return failAuth + } + return failure(status, body) +} + // openRouterSharedPool says an OpenRouter free model was refused by the // provider's shared pool, rather than by OpenRouter's account-wide free tier. func openRouterSharedPool(body []byte) bool { @@ -311,7 +319,7 @@ func (s *Server) restAfter(c candidate, status int, header http.Header, body []b func (s *Server) restAfterMarked(c candidate, status int, header http.Header, body []byte, sharedPool bool) Rest { now := time.Now() d := fallbackCooldown - why := failure(status, body) + why := failureOf(c, status, body) r := Rest{Why: why, Status: status, By: "cooldown"} if why == failProxy { // the account is as good as it was; the proxy is the user's to start diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index caca1303b..7cf279dc2 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -7858,7 +7858,11 @@ function renderAccounts(a, p) { const sub = subOf(a.agent); const list = el("div", "accts"); const ls = loginsInOrder(a, p); - const several = ls.filter((l) => !l.lapsed && ((l.active && !l.paused) || l.on)).length > 1; + // a Claude account whose sign-in Anthropic refused, or that has none, + // can't be used till it is signed in again; another subscription's lapse + // shows on its quota line only, as before + const unusable = (l) => a.agent === "claude" && !!l.lapsed; + const several = ls.filter((l) => !unusable(l) && ((l.active && !l.paused) || l.on)).length > 1; // kept signed in to one of the user's choosing (#524), the first is the // first in use in the order, which Make first sets without a sign-in const kept = keptLogin(p); @@ -7872,18 +7876,18 @@ function renderAccounts(a, p) { // the account Claude Code or Codex is signed in to can be paused while // another is on: the gateway passes over it, the agent staying signed in // to it (#263) - const pausable = (a.agent === "claude" || a.agent === "codex") && ls.some((l) => !l.lapsed && !l.active && l.on); + const pausable = (a.agent === "claude" || a.agent === "codex") && ls.some((l) => !unusable(l) && !l.active && l.on); const quota = loginUsageOf(a.agent); // the first, which magpie signed the agent out of while it was spent: // it is signed back in once it has room (#408) const back = ls.find((l) => l.returns && !l.active); for (const l of ls) { - const on = !l.lapsed && !l.paused && (l.active || l.on); + const on = !unusable(l) && !l.paused && (l.active || l.on); const row = el("div", "acc" + (on ? " in-use" : " off") + (l.user === justAdded ? " new" : "")); row.dataset.accountId = l.user; const dot = el("button", "dot tick"); if (on) dot.append(svg(CHECK, 10, 2.2)); - if (l.lapsed) { + if (unusable(l)) { dot.disabled = true; dot.title = t("Sign in again to use this account"); } else if (l.active && (pausable || l.paused)) { @@ -7901,13 +7905,17 @@ function renderAccounts(a, p) { const [amPill, amBox] = ls.length > 1 || accountModelsOf(p, l.user).length ? accountModels(p, l.user, false, l.user) : []; if (amPill) row.append(amPill); row.append(el("span", "grow")); - if (l.lapsed) { + if (unusable(l)) { row.append(el("span", "using", t("Sign-in required"))); const again = el("button", "text", t("Sign in again")); again.onclick = () => startSignIn(a.agent); - const forget = el("button", "text quiet", t("Remove")); - forget.onclick = () => accountAction("login/forget", { agent: a.agent, user: l.user }, t("{user} removed", { user: l.user })); - row.append(again, forget); + row.append(again); + // the one Claude Code is signed in to can't be forgotten (ForgetLogin) + if (!l.active) { + const forget = el("button", "text quiet", t("Remove")); + forget.onclick = () => accountAction("login/forget", { agent: a.agent, user: l.user }, t("{user} removed", { user: l.user })); + row.append(forget); + } } else if (l.active && kept && l.user !== firstUser) { // signed in to, kept so, and tried at its place in the order const signed = el("span", "using", l.paused ? t("Paused") : t("Signed in")); diff --git a/internal/gui/assets/i18n.js b/internal/gui/assets/i18n.js index 53b806569..ea4961c58 100644 --- a/internal/gui/assets/i18n.js +++ b/internal/gui/assets/i18n.js @@ -932,6 +932,7 @@ const I18N = { "sign-in required": "需要重新登录", "Sign in again to use this account": "重新登录后才能使用此账号", "Claude Code is no longer signed in; sign in again in magpie": "Claude Code 已没有有效登录，请在 magpie 中重新登录", + "its sign-in is gone; sign in again": "登录信息已不存在，请重新登录", "Claude Code could not authenticate this account; sign in again in magpie": "Claude Code 无法认证此账号，请在 magpie 中重新登录", "Tested {user}: {result}": "已测试 {user}：{result}", "{who} could not authenticate. Sign in again in magpie; this login is not retried. The request went to {next}.": "{who} 认证失败。请在 magpie 中重新登录；当前登录不再重试。请求已转给 {next}。", diff --git a/internal/gui/tests/claude-auth-status.test.cjs b/internal/gui/tests/claude-auth-status.test.cjs index 8c1d6ceff..9d72f1068 100644 --- a/internal/gui/tests/claude-auth-status.test.cjs +++ b/internal/gui/tests/claude-auth-status.test.cjs @@ -29,15 +29,29 @@ const provider = { { user: expired.who, plan: "enterprise", on: true, lapsed: lapse }, ] }, }; +// Claude Code still signed in to the refused account: it can't be removed +const signedInRefused = { ...provider, account: { ...provider.account, user: expired.who, logins: [ + { user: expired.who, plan: "enterprise", active: true, on: true, lapsed: lapse }, + { user: healthy.who, plan: "max", on: true }, +] } }; +// another subscription's lapse keeps its row as it was +const codex = { + id: "codex", name: "Codex", icon: "codex-color", responses: "https://api.example.test", chat: "", anthropic: "", catalog: "", + models: [{ id: "gpt-6-luna", on: true }], agents: [], fallback: [], headers: {}, keyList: [], proxy: "", + account: { agent: "codex", agentName: "Codex", user: "a@example.com", plan: "plus", logins: [ + { user: "a@example.com", plan: "plus", active: true, on: true }, + { user: "b@example.com", plan: "plus", on: true, lapsed: "refresh token reused" }, + ] }, +}; -function serve(lang) { +function serve(lang, providers = [provider]) { return async (route) => { const url = new URL(route.request().url()); const json = (data) => route.fulfill({ json: data }); if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs={lang:"${lang}",theme:"light",web:true};` }); if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" }); if (url.pathname === "/api/state") return json({ agents: [{ id: "codex", name: "Codex", fields: [] }], profiles: [], settings: { lang, theme: "light" } }); - if (url.pathname === "/api/providers") return json({ providers: [provider], presets: [], excluded: [], gateway: { running: true, window: true } }); + if (url.pathname === "/api/providers") return json({ providers, presets: [], excluded: [], gateway: { running: true, window: true } }); if (url.pathname === "/api/provider/test") return json({ results: [{ protocol: "anthropic", ok: true, ms: 300, model: healthy.model, account: healthy.who }], provider }); if (url.pathname === "/api/gateway/trace") { if (url.searchParams.get("wait")) await new Promise((r) => setTimeout(r, 20e3)); @@ -104,3 +118,33 @@ for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", }); } } + +for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) { + for (const lang of ["en", "zh"]) { + test(`${engine} ${lang}: only Claude rows need a new sign-in, and the signed-in one can't be removed`, async (t) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + t.after(() => browser.close()); + const page = await (await browser.newContext({ viewport: { width: 1100, height: 800 }, reducedMotion: "reduce" })).newPage(); + const errors = []; + page.on("pageerror", (e) => errors.push(e.message)); + page.setDefaultTimeout(5000); + await page.route("**/*", serve(lang, [signedInRefused, codex])); + await page.goto("http://magpie.test/?view=providers"); + await page.locator(".row.provider", { hasText: "Claude Code" }).first().click(); + const own = page.locator(`.editor .acc[data-account-id="${expired.who}"]`); + await own.waitFor(); + assert.equal(await own.locator(".dot").isDisabled(), true); + await own.getByRole("button", { name: lang === "zh" ? "重新登录" : "Sign in again", exact: true }).waitFor(); + assert.equal(await own.getByRole("button", { name: lang === "zh" ? "移除" : "Remove", exact: true }).count(), 0, "Claude Code's own account can't be forgotten"); + + await page.goto("http://magpie.test/?view=providers"); + await page.locator(".row.provider", { hasText: "Codex" }).first().click(); + const other = page.locator('.editor .acc[data-account-id="b@example.com"]'); + await other.waitFor(); + assert.equal(await other.locator(".dot").isDisabled(), false, "a Codex account's dot still turns it off"); + assert.equal(await other.locator(".dot svg").count(), 1, "a Codex account on is shown on"); + assert.equal(await other.getByText(lang === "zh" ? "需要重新登录" : "Sign-in required", { exact: true }).count(), 0); + assert.deepEqual(errors, []); + }); + } +}