Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/unreleased/workbuddy-401-refresh-false-auth.md
Original file line number Diff line number Diff line change
@@ -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 记下可诊断原因。
51 changes: 42 additions & 9 deletions internal/providers/workbuddy/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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
Expand Down
80 changes: 79 additions & 1 deletion internal/providers/workbuddy/upstream_align_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"

Expand Down Expand Up @@ -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:
Expand All @@ -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) {
Expand Down
Loading