Skip to content

fix: 串行化 token 刷新,修复并发竞态搞坏用户凭证 - #2

Open
charleshan7 wants to merge 1 commit into
agent/widget-monochrome-codex-weekly-windowfrom
fix/refresh-token-race
Open

fix: 串行化 token 刷新,修复并发竞态搞坏用户凭证#2
charleshan7 wants to merge 1 commit into
agent/widget-monochrome-codex-weekly-windowfrom
fix/refresh-token-race

Conversation

@charleshan7

@charleshan7 charleshan7 commented Jul 28, 2026

Copy link
Copy Markdown
Owner

问题

小组件的 Claude 数据反复卡在旧值不更新,用户先后被迫重新登录三次。

根因

Agent 的并发刷新竞态。服务是 ThreadingHTTPServer,而 reloadAllTimelines() 会同时唤醒小/中/大三个组件来取数,但 Snapshot.get()read_claude() 都没有锁。多个线程各自拿同一个 refreshToken 去刷新,而每次刷新都会轮换 refreshToken——后发的那次让先写回的那份当场作废,Keychain 里就留下一个死 token,用户只能重新登录。(claude 自己也在刷同一份凭证,是第二个对撞源。)

修复

  • _REFRESH_LOCK 串行化刷新,锁内重读 Keychain 捡起别人刚写回的新 token
  • Snapshot.get() 双检锁,从源头掐掉并发取数
  • 刷新被拒时重读 Keychain 自愈:claude 若已写回新 token 就直接用,免去重登
  • refreshToken 为空时提前拦截(claude 刷新失败后会清空它)
  • 报错带上失败阶段和响应体;凭证真失效时下发 relogin 标记

验证

agent/tests/ 两个回归测试,模拟服务端的 refreshToken 轮换语义:

刷新调用 被服务端拒绝 8 个并发请求成功数
修复前 8 次 7 次 1
修复后 1 次 0 次 8

线上验证:Agent 恢复实时取数,stale/relogin 标记均已消失。

附带

  • UIstale 时卡片显示「已过期」角标(relogin 时显示「需重新登录」)+ 数据变灰;菜单栏面板区分「登录失效去 /login」和「临时取不到」。此前过期数据和正常数据长得一模一样,只能靠肉眼比对重置时间才能发现——这是问题拖了这么久的直接原因。
  • UsageViews.swift 补上漏掉的 import WidgetKit(原先靠 Release 全模块优化才编得过,Debug 会炸)
  • __pycache__ 移出版本控制
  • AGENTS.md 记录本次及相关踩坑(含「重登必须在 Terminal.app 里用独立 CLI」——桌面版内嵌的 Claude Code 认证由桌面版托管,在里面 /login 一个字节都不写 Claude Code-credentials

基于 #1 分支,待 #1 合并后 GitHub 会自动改指向 main。

🤖 Generated with Claude Code


Summary by cubic

串行化 Claude OAuth 刷新并为取数加锁,修复并发竞态导致的 Keychain 凭证被轮换作废,恢复小组件实时取数并消除反复要求重登。

  • Bug Fixes
    • 串行刷新:新增 _REFRESH_LOCK,锁内重读 Keychain 拿最新 token;refreshToken 为空直接拦截。
    • 自愈刷新失败:刷新遇到 400/401 时重读 Keychain,若已有新 token 直接用;仅在凭证确实失效时标记 relogin。错误消息带阶段与响应体。
    • 并发取数防抖:Snapshot.get() 双检锁,减少并发调用;兜底使用上次成功值时保留 error/relogin 标记。
    • 回归测试:新增 test_refresh_race.pytest_refresh_selfheal.py,验证刷新只发 1 次、0 拒绝,8/8 请求成功。
    • 其他:UI 显示「已过期/需重新登录」角标 + 过期变灰;面板提示重登路径;Shared/UsageViews.swift 增加 import WidgetKit``;.gitignore 忽略 `pycache/` 与 `*.pyc`;新增 `AGENTS.md` 记录架构与踩坑。

Written for commit a25c141. Summary will update on new commits.

Review in cubic

小组件 Claude 数据反复卡在旧值不更新,根因是 Agent 的并发刷新竞态。

服务是 ThreadingHTTPServer,而 reloadAllTimelines() 会同时唤醒小/中/大
三个组件来取数,Snapshot.get() 和 read_claude() 都没有锁。多个线程各自拿
同一个 refreshToken 去刷新,而每次刷新都会轮换 refreshToken——后发的那次
让先写回的那份当场作废,Keychain 里留下一个死 token,用户只能重新登录。
(claude 自己也在刷同一份凭证,是第二个对撞源。)

修复:
- _REFRESH_LOCK 串行化刷新,并在锁内重读 Keychain,捡起别人刚写回的新
  token,避免重复消耗
- Snapshot.get() 加双检锁,从源头掐掉并发取数
- 刷新被拒时重读 Keychain 自愈:claude 若已写回新 token 就直接用,免去重登
- refreshToken 为空时提前拦截(claude 刷新失败后会清空它),不再发那个只会
  换来 "Invalid request format" 的请求
- 报错带上失败阶段和响应体,凭证真失效时下发 relogin 标记

回归测试 agent/tests/:修复前跑 test_refresh_race 是 8 次刷新 7 次被拒,
修复后 1 次刷新全部成功。

UI:stale 时卡片显示「已过期」角标(relogin 时显示「需重新登录」)+ 数据
变灰,菜单栏面板区分「登录失效去 /login」和「临时取不到」。此前过期数据
和正常数据长得一模一样,只能靠肉眼比对重置时间才能发现。

另:UsageViews.swift 补上漏掉的 import WidgetKit(原先靠 Release 全模块
优化才编得过,Debug 会炸);__pycache__ 移出版本控制;AGENTS.md 记录本次
及相关踩坑。

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a25c14123f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread agent/usage-agent.py
Comment on lines +695 to +697
val["error"] = fail.get("error")
if fail.get("relogin"):
val["relogin"] = True

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve refresh details in stale payloads

When Claude has a last-good value and a refresh fails with HTTP 400/401, read_claude() supplies the response body in detail, but this fallback copies only error and relogin into the stale result. Consequently, the normal expired-token path makes /usage collapse both Refresh token expired and Refresh token not found or invalid into the same generic message, preventing the documented diagnosis of expiration versus token-rotation corruption; preserve detail (and the failure stage) in the stale payload.

AGENTS.md reference: AGENTS.md:L34-L34

Useful? React with 👍 / 👎.

Comment thread App/UsageApp.swift
}

// 取数凭证失效时 Agent 会回吐缓存(stale),提示怎么恢复
if snapshot.claude.stale == true || snapshot.codex.stale == true {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Show re-login guidance even without cached data

When credentials are already invalid on a fresh installation or after last-good.json is removed, Snapshot returns relogin: true without stale, so this condition suppresses the recovery panel. The user then sees only the generic product error and not the required instruction to authenticate through the standalone CLI in Terminal.app; include relogin in the condition so this path cannot direct users toward the ineffective Claude Desktop login flow.

AGENTS.md reference: AGENTS.md:L35-L35

Useful? React with 👍 / 👎.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 issues found across 9 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="App/UsageApp.swift">

<violation number="1" location="App/UsageApp.swift:56">
P2: A credential failure with no last-good snapshot does not show the new menu-bar re-login guidance: `Snapshot` leaves `stale` unset when there is no cache, while this banner is entered only for `stale`. Including the `relogin` state in the no-cache UI path would preserve the distinction between login failure and a temporary fetch failure.</violation>
</file>

<file name="Shared/UsageViews.swift">

<violation number="1" location="Shared/UsageViews.swift:215">
P2: Widget stale state only says “需重新登录”, so users viewing the widget are not told to run `/login` through the independent Terminal.app CLI; using the desktop app’s embedded Claude Code will not update the credentials read by the Agent. Add a visible recovery instruction or a widget-accessible path to that instruction for `relogin`.</violation>
</file>

<file name="agent/usage-agent.py">

<violation number="1" location="agent/usage-agent.py:297">
P1: Agent 先刷新成功而 Claude 已携旧 token 发出刷新时,凭证仍会被 Claude 的失败处理覆盖:`threading.Lock` 只串行本进程线程,无法协调 Claude。建议覆盖这一方向的并发恢复(成功写回后检测并恢复外部清空的凭证),或采用 Claude 也参与的跨进程协调;当前重读逻辑只在 Agent 自己先收到拒绝时自愈。</violation>

<violation number="2" location="agent/usage-agent.py:695">
P2: The stale-fallback block only forwards `error` and `relogin` from the failed refresh result, dropping the `detail` field that `read_claude()` populates for HTTP 400/401 failures. As a result, `/usage` can't distinguish an expired refresh token from a rotated/invalidated one once the stale cache path kicks in — consider also copying `fail.get("detail")` into the stale payload.</violation>
</file>

<file name="agent/tests/test_refresh_selfheal.py">

<violation number="1" location="agent/tests/test_refresh_selfheal.py:14">
P3: The self-heal regression test does not actually test recovery from a rejected refresh: its lock-time reread replaces `rt-dead` with `rt-live` before the first refresh attempt. Keeping the dead token through the second read and returning `rt-live` only on the retry read would cover the behavior described by the test.</violation>
</file>

<file name="agent/tests/test_refresh_race.py">

<violation number="1" location="agent/tests/test_refresh_race.py:60">
P3: This regression test validates `_REFRESH_LOCK`, but not the `Snapshot.get()` serialization that is also part of the fix. Exercising concurrent `Snapshot.get()` calls (and counting the underlying fetch/read calls) would make the test fail if duplicate requests return.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread agent/usage-agent.py
# refreshToken 每次刷新都会轮换:并发刷新会让先写回的那份当场作废,凭证永久坏掉
# (只能重新登录)。所有刷新必须串行,且在锁内重读 Keychain——别的线程或 claude
# 自己可能刚刷过,重读后往往已无需再刷。
_REFRESH_LOCK = threading.Lock()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Agent 先刷新成功而 Claude 已携旧 token 发出刷新时,凭证仍会被 Claude 的失败处理覆盖:threading.Lock 只串行本进程线程,无法协调 Claude。建议覆盖这一方向的并发恢复(成功写回后检测并恢复外部清空的凭证),或采用 Claude 也参与的跨进程协调;当前重读逻辑只在 Agent 自己先收到拒绝时自愈。

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At agent/usage-agent.py, line 297:

<comment>Agent 先刷新成功而 Claude 已携旧 token 发出刷新时,凭证仍会被 Claude 的失败处理覆盖:`threading.Lock` 只串行本进程线程,无法协调 Claude。建议覆盖这一方向的并发恢复(成功写回后检测并恢复外部清空的凭证),或采用 Claude 也参与的跨进程协调;当前重读逻辑只在 Agent 自己先收到拒绝时自愈。</comment>

<file context>
@@ -291,24 +291,70 @@ def _refresh(opener, refresh_token):
+# refreshToken 每次刷新都会轮换:并发刷新会让先写回的那份当场作废,凭证永久坏掉
+# (只能重新登录)。所有刷新必须串行,且在锁内重读 Keychain——别的线程或 claude
+# 自己可能刚刷过,重读后往往已无需再刷。
+_REFRESH_LOCK = threading.Lock()
+
+class ReloginRequired(Exception):
</file context>

Comment thread App/UsageApp.swift
}

// 取数凭证失效时 Agent 会回吐缓存(stale),提示怎么恢复
if snapshot.claude.stale == true || snapshot.codex.stale == true {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A credential failure with no last-good snapshot does not show the new menu-bar re-login guidance: Snapshot leaves stale unset when there is no cache, while this banner is entered only for stale. Including the relogin state in the no-cache UI path would preserve the distinction between login failure and a temporary fetch failure.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At App/UsageApp.swift, line 56:

<comment>A credential failure with no last-good snapshot does not show the new menu-bar re-login guidance: `Snapshot` leaves `stale` unset when there is no cache, while this banner is entered only for `stale`. Including the `relogin` state in the no-cache UI path would preserve the distinction between login failure and a temporary fetch failure.</comment>

<file context>
@@ -52,6 +52,27 @@ struct ContentView: View {
             }
 
+            // 取数凭证失效时 Agent 会回吐缓存(stale),提示怎么恢复
+            if snapshot.claude.stale == true || snapshot.codex.stale == true {
+                VStack(alignment: .leading, spacing: 4) {
+                    if snapshot.claude.stale == true {
</file context>

Comment thread Shared/UsageViews.swift
var plan: String? = nil
var stale: Bool = false
var relogin: Bool = false
private var staleLabel: String { relogin ? "需重新登录" : "已过期" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Widget stale state only says “需重新登录”, so users viewing the widget are not told to run /login through the independent Terminal.app CLI; using the desktop app’s embedded Claude Code will not update the credentials read by the Agent. Add a visible recovery instruction or a widget-accessible path to that instruction for relogin.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Shared/UsageViews.swift, line 215:

<comment>Widget stale state only says “需重新登录”, so users viewing the widget are not told to run `/login` through the independent Terminal.app CLI; using the desktop app’s embedded Claude Code will not update the credentials read by the Agent. Add a visible recovery instruction or a widget-accessible path to that instruction for `relogin`.</comment>

<file context>
@@ -209,6 +210,9 @@ struct ProductHeader: View {
     var plan: String? = nil
+    var stale: Bool = false
+    var relogin: Bool = false
+    private var staleLabel: String { relogin ? "需重新登录" : "已过期" }
     var body: some View {
         HStack(spacing: 6) {
</file context>

Comment thread agent/usage-agent.py
val = dict(self.last_good[key])
val["stale"] = True
# 兜底展示旧数据,但保留失败原因:需重新登录要能提示到用户
val["error"] = fail.get("error")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The stale-fallback block only forwards error and relogin from the failed refresh result, dropping the detail field that read_claude() populates for HTTP 400/401 failures. As a result, /usage can't distinguish an expired refresh token from a rotated/invalidated one once the stale cache path kicks in — consider also copying fail.get("detail") into the stale payload.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At agent/usage-agent.py, line 695:

<comment>The stale-fallback block only forwards `error` and `relogin` from the failed refresh result, dropping the `detail` field that `read_claude()` populates for HTTP 400/401 failures. As a result, `/usage` can't distinguish an expired refresh token from a rotated/invalidated one once the stale cache path kicks in — consider also copying `fail.get("detail")` into the stale payload.</comment>

<file context>
@@ -617,8 +688,13 @@ def get(self, opener):
                 val = dict(self.last_good[key])
                 val["stale"] = True
+                # 兜底展示旧数据,但保留失败原因:需重新登录要能提示到用户
+                val["error"] = fail.get("error")
+                if fail.get("relogin"):
+                    val["relogin"] = True
</file context>
Suggested change
val["error"] = fail.get("error")
val["error"] = fail.get("error")
val["detail"] = fail.get("detail")

# 第一次读到的是已作废的 rt-dead;claude 随后写回了 rt-live
def fake_read(acct):
state["reads"] += 1
rt = "rt-dead" if state["reads"] == 1 else "rt-live"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The self-heal regression test does not actually test recovery from a rejected refresh: its lock-time reread replaces rt-dead with rt-live before the first refresh attempt. Keeping the dead token through the second read and returning rt-live only on the retry read would cover the behavior described by the test.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At agent/tests/test_refresh_selfheal.py, line 14:

<comment>The self-heal regression test does not actually test recovery from a rejected refresh: its lock-time reread replaces `rt-dead` with `rt-live` before the first refresh attempt. Keeping the dead token through the second read and returning `rt-live` only on the retry read would cover the behavior described by the test.</comment>

<file context>
@@ -0,0 +1,50 @@
+# 第一次读到的是已作废的 rt-dead;claude 随后写回了 rt-live
+def fake_read(acct):
+    state["reads"] += 1
+    rt = "rt-dead" if state["reads"] == 1 else "rt-live"
+    return {"claudeAiOauth": {"accessToken": "at-old", "refreshToken": rt,
+                              "expiresAt": 0, "subscriptionType": "pro"}}
</file context>

# ---- 8 个线程同时取数(= reloadAllTimelines 唤醒小/中/大 + 菜单栏 App)----
results = []
def worker():
results.append(m.read_claude(None))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This regression test validates _REFRESH_LOCK, but not the Snapshot.get() serialization that is also part of the fix. Exercising concurrent Snapshot.get() calls (and counting the underlying fetch/read calls) would make the test fail if duplicate requests return.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At agent/tests/test_refresh_race.py, line 60:

<comment>This regression test validates `_REFRESH_LOCK`, but not the `Snapshot.get()` serialization that is also part of the fix. Exercising concurrent `Snapshot.get()` calls (and counting the underlying fetch/read calls) would make the test fail if duplicate requests return.</comment>

<file context>
@@ -0,0 +1,84 @@
+# ---- 8 个线程同时取数(= reloadAllTimelines 唤醒小/中/大 + 菜单栏 App)----
+results = []
+def worker():
+    results.append(m.read_claude(None))
+
+threads = [threading.Thread(target=worker) for _ in range(8)]
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant