diff --git a/changelog/unreleased/workbuddy-401-refresh-false-auth.md b/changelog/unreleased/workbuddy-401-refresh-false-auth.md new file mode 100644 index 00000000..992db28b --- /dev/null +++ b/changelog/unreleased/workbuddy-401-refresh-false-auth.md @@ -0,0 +1,7 @@ +### English + +- WorkBuddy chat no longer treats every post-401 token refresh failure as `login_required`: only real session-dead / auth refresh failures keep that path; transport and 5xx refresh errors stay `unavailable` with a short pool cooldown and a diagnostic Observe entry. + +### 中文 + +- WorkBuddy 聊天在 401 后刷新 token 失败时不再一律标成 `login_required`:仅真实 session-dead / auth 刷新失败保留该路径;运输错误与 5xx 刷新失败保持 `unavailable`,走池短冷却,并用现有 Observe 记下可诊断原因。 diff --git a/internal/providers/workbuddy/client.go b/internal/providers/workbuddy/client.go index 03f8e7c9..617f1ef4 100644 --- a/internal/providers/workbuddy/client.go +++ b/internal/providers/workbuddy/client.go @@ -522,11 +522,13 @@ func (c *Client) ChatStream(ctx context.Context, accountID string, req translate // sendChat performs one chat round trip and returns a 2xx response with an // unread body. A 401 usually means the access token expired before its JWT -// clock said so: refresh the token once and retry on the same account. Only -// when the refresh itself fails is the 401 surfaced as KindAuth, which lets -// the executor fail over to another account (Refresh already marks a dead -// session as login_required). TLS certificate verification failures are -// deterministic configuration errors and are reported without retry. +// clock said so: refresh the token once and retry on the same account. When +// refresh itself fails, only a real KindAuth / session-dead failure is +// surfaced as KindAuth (Refresh already Observe(login_required) there); +// transport / 5xx / other non-auth refresh failures return KindUnavailable +// so the pool applies short cooldown + failover without marking the account +// login_required. TLS certificate verification failures are deterministic +// configuration errors and are reported without retry. func (c *Client) sendChat(ctx context.Context, accountID string, credential Credential, req translate.ChatRequest, httpReq *http.Request) (*http.Response, error) { client, err := c.httpClient(ctx, accountID) if err != nil { @@ -553,11 +555,22 @@ func (c *Client) sendChat(ctx context.Context, accountID string, credential Cred if tlsErr := tlsVerifyError(refreshErr); tlsErr != nil { return nil, tlsErr } - out := classifiedError(resp.StatusCode, body) - if pe, ok := out.(*providers.Error); ok && pe.Kind == accounts.KindAuth { - pe.Message = "token refresh after 401 failed; re-login required" + if refreshFailureIsAuth(refreshErr) { + out := classifiedError(resp.StatusCode, body) + if pe, ok := out.(*providers.Error); ok && pe.Kind == accounts.KindAuth { + pe.Message = "token refresh after 401 failed; re-login required" + } + return nil, out + } + // Non-auth refresh failure (transport / 5xx / etc.): keep the + // account out of login_required; pool short-cools via Unavailable. + summary := refreshErr.Error() + _ = c.store.Observe(ctx, accountID, credential.UID, "error", "token refresh after 401: "+summary, accounts.KindUnavailable) + return nil, &providers.Error{ + Kind: accounts.KindUnavailable, + Status: http.StatusBadGateway, + Message: "token refresh after 401 unavailable: " + summary, } - return nil, out } credential = fresh httpReq, _, err = c.chatRequest(ctx, accountID, credential, req) @@ -572,6 +585,26 @@ func (c *Client) sendChat(ctx context.Context, accountID string, credential Cred // account or transient network fault. const tlsVerifyHint = "TLS certificate verification failed: the upstream certificate chain is not trusted by this host (corporate proxy / HTTPS-decrypting VPN?). Install the issuing root CA into the system trust store or set SSL_CERT_FILE, then retry." +// refreshFailureIsAuth reports whether a Refresh error is a real session-dead +// / KindAuth failure. Reuses Classify and the existing session-dead markers; +// Refresh already Observe(login_required) on that path. Non-auth failures +// (transport, 5xx, envelope) must not be upgraded to KindAuth in sendChat. +func refreshFailureIsAuth(err error) bool { + if err == nil { + return false + } + var pe *providers.Error + if errors.As(err, &pe) && pe.Kind == accounts.KindAuth { + return true + } + msg := err.Error() + // Refresh returns "workbuddy session dead: re-login required" for Classify KindAuth. + if strings.Contains(strings.ToLower(msg), "session dead") { + return true + } + return Classify(0, msg).Kind == accounts.KindAuth +} + func tlsVerifyError(err error) error { if !providers.IsTLSVerifyError(err) { return nil diff --git a/internal/providers/workbuddy/upstream_align_test.go b/internal/providers/workbuddy/upstream_align_test.go index 3b232a1d..9a6122ee 100644 --- a/internal/providers/workbuddy/upstream_align_test.go +++ b/internal/providers/workbuddy/upstream_align_test.go @@ -7,6 +7,7 @@ import ( "errors" "net/http" "net/http/httptest" + "strings" "sync/atomic" "testing" @@ -70,7 +71,7 @@ func TestChat401RefreshesOnceAndRetriesSameAccount(t *testing.T) { func TestChat401RefreshFailureSurfacesAuthForFailover(t *testing.T) { var chats, refreshes int32 - client, _ := newAlignClient(t, Credential{AccessToken: "old", RefreshToken: "rt", UID: "u1", Domain: DomainCN, ExpiresAt: 4102444800}, + client, store := newAlignClient(t, Credential{AccessToken: "old", RefreshToken: "rt", UID: "u1", Domain: DomainCN, ExpiresAt: 4102444800}, func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case pathProductConfig: @@ -88,9 +89,86 @@ func TestChat401RefreshFailureSurfacesAuthForFailover(t *testing.T) { if !errors.As(err, &pe) || pe.Kind != accounts.KindAuth { t.Fatalf("want KindAuth after failed refresh, got %v", err) } + if pe.Message != "token refresh after 401 failed; re-login required" { + t.Fatalf("auth message = %q", pe.Message) + } + if chats != 1 || refreshes != 1 { + t.Fatalf("chats=%d refreshes=%d; refresh must run once and not retry chat", chats, refreshes) + } + // Refresh Observe(login_required); sendChat must not Observe unavailable. + if store.lastStatus != "login_required" || store.lastKind != accounts.KindAuth { + t.Fatalf("want Refresh Observe(login_required/auth), got status=%q kind=%q", store.lastStatus, store.lastKind) + } +} + +func TestChat401RefreshNonAuthSurfacesUnavailable(t *testing.T) { + var chats, refreshes int32 + client, store := newAlignClient(t, Credential{AccessToken: "old", RefreshToken: "rt", UID: "u1", Domain: DomainCN, ExpiresAt: 4102444800}, + func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case pathProductConfig: + catalogOK(w) + case pathTokenRefresh: + atomic.AddInt32(&refreshes, 1) + w.WriteHeader(http.StatusBadGateway) + _, _ = w.Write([]byte(`upstream boom`)) + case pathChat: + atomic.AddInt32(&chats, 1) + w.WriteHeader(http.StatusUnauthorized) + } + }) + _, _, err := client.ChatStream(context.Background(), "acc1", alignReq) + var pe *providers.Error + if !errors.As(err, &pe) || pe.Kind != accounts.KindUnavailable { + t.Fatalf("want KindUnavailable after non-auth refresh failure, got %v", err) + } + if strings.Contains(pe.Message, "re-login required") { + t.Fatalf("non-auth refresh must not surface re-login message: %q", pe.Message) + } + if !strings.Contains(pe.Message, "refresh") { + t.Fatalf("want refresh failure summary in message, got %q", pe.Message) + } if chats != 1 || refreshes != 1 { t.Fatalf("chats=%d refreshes=%d; refresh must run once and not retry chat", chats, refreshes) } + if store.lastKind == accounts.KindAuth || store.lastStatus == "login_required" { + t.Fatalf("non-auth refresh must not Observe login_required/auth: status=%q kind=%q", store.lastStatus, store.lastKind) + } + if store.lastKind != accounts.KindUnavailable || store.lastStatus != "error" { + t.Fatalf("want Observe(error/unavailable) diagnostic, got status=%q kind=%q", store.lastStatus, store.lastKind) + } +} + +func TestChat401RefreshSessionDeadBodySurfacesAuth(t *testing.T) { + var chats, refreshes int32 + client, store := newAlignClient(t, Credential{AccessToken: "old", RefreshToken: "rt", UID: "u1", Domain: DomainCN, ExpiresAt: 4102444800}, + func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case pathProductConfig: + catalogOK(w) + case pathTokenRefresh: + atomic.AddInt32(&refreshes, 1) + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{ + "code": sessionDeadCode, + "msg": sessionDeadText, + }) + case pathChat: + atomic.AddInt32(&chats, 1) + w.WriteHeader(http.StatusUnauthorized) + } + }) + _, _, err := client.ChatStream(context.Background(), "acc1", alignReq) + var pe *providers.Error + if !errors.As(err, &pe) || pe.Kind != accounts.KindAuth { + t.Fatalf("want KindAuth for session-dead refresh body, got %v", err) + } + if store.lastStatus != "login_required" || store.lastKind != accounts.KindAuth { + t.Fatalf("want Refresh Observe(login_required/auth), got status=%q kind=%q", store.lastStatus, store.lastKind) + } + if chats != 1 || refreshes != 1 { + t.Fatalf("chats=%d refreshes=%d", chats, refreshes) + } } func TestTLSVerifyErrorIsWrappedNotRetried(t *testing.T) {