Skip to content

refactor(settings): 移除无人消费的 NEKO_SERVERS_DESKTOP_CLIENT_ID 第二真相源 - #2517

Merged
wehos merged 2 commits into
mainfrom
fix/desktop-client-id-single-source
Jul 28, 2026
Merged

refactor(settings): 移除无人消费的 NEKO_SERVERS_DESKTOP_CLIENT_ID 第二真相源#2517
wehos merged 2 commits into
mainfrom
fix/desktop-client-id-single-source

Conversation

@wehos

@wehos wehos commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

#1542 收尾第 4 批。

问题

#1542plugin/settings.py 里定义了 NEKO_SERVERS_DESKTOP_CLIENT_ID,并同时加进 __all__PUBLIC_SYSTEM_CONFIG_KEYS(后者会经 PLUGIN_SYSTEM_CONFIG_GET 暴露给任何已装插件读取)。

但真正消费这个配置的 main_routers/community_oauth.py:84 是直接读 os.environ,从未 import 过这个常量。两份的兜底逻辑还不一致:

位置 逻辑
plugin/settings.py os.getenv(..., "neko-servers-desktop-dev").strip() or 默认值
main_routers/community_oauth.py 多一层 raw != "neko-desktop" 防误配

也就是说改 settings 那份不会有任何效果 —— 是个会误导后来人的埋雷。

为什么是删掉而不是反向统一

scripts/check_module_layering.py 定义 L3 = main_routers、L4 = plugin,低层不能 import 高层。现有 main_routers/ 下确实零个 from plugin import。所以不能让 community_oauth 去读 plugin.settings —— 唯一方向是删掉 settings 那份。

这也符合语义:该值是桌面端社区 OAuth 的 client id,插件用不到,本就不属于"插件公共配置"(PUBLIC_SYSTEM_CONFIG_KEYS 里的 NEKO_AUTH_CLIENT_ID 是插件市场的,那个才需要暴露)。

改动

  • 删除 NEKO_SERVERS_DESKTOP_CLIENT_ID 的常量定义、__all__ 条目、PUBLIC_SYSTEM_CONFIG_KEYS 条目
  • 原注释里的防误配意图改挂到 NEKO_AUTH_CLIENT_ID 上,并指明该值归 main_routers/community_oauth.py 所有
  • 新增 test_desktop_client_id_is_owned_here_and_rejects_plugin_market_client,把"误配成 neko-desktop 时必须回落到 servers-desktop 默认值"这条意图钉死 —— 原先只有 assert "neko-desktop" not in body["auth_url"] 的间接覆盖

回归报告 / Regression Report

  • 改动了什么:移除 plugin/settings.py 中无人消费的 NEKO_SERVERS_DESKTOP_CLIENT_ID 定义与两处注册;补一条防误配的直接守卫测试。
  • 理由 / 必要性:第二真相源且永不生效,改它无效果;同时它把一个插件用不到的值暴露给了所有已装插件。
  • 改动前后的表现对比:无行为差异 —— 该常量零消费,community_oauth 的解析逻辑一行未动。唯一变化是插件通过 PLUGIN_SYSTEM_CONFIG_GET 拿到的字典少一个 key(该 key 无插件读取)。
  • 潜在回归点:若某个第三方插件依赖从系统配置里读到这个 key —— 已 grep 全仓(.py / .ts / .tsx / .js)确认无消费者;plugin/tests 中对 PUBLIC_SYSTEM_CONFIG_KEYS 的测试是整体 monkeypatch 替换元组、不钉具体成员,不受影响。
  • 验证:变异验证 —— 删掉 community_oauthraw != "neko-desktop" 后新测试立刻变红,还原后 29 passed;uv run pytest tests/unit -q6791 passed, 26 skippeduv run pytest plugin/tests -q2752 passed, 20 skipped

不拆分理由 / Why Not Split

删除定义与补上守卫测试是同一件事的两半:删掉那份"文档式常量"后,它承载的防误配意图需要由测试接住,否则下次有人简化 _desktop_client_id() 就没人拦。共 2 个文件、+28/-12。

#1542 在 plugin/settings.py 里定义了 NEKO_SERVERS_DESKTOP_CLIENT_ID,并把它同时
加进 __all__ 与 PUBLIC_SYSTEM_CONFIG_KEYS(后者会经 PLUGIN_SYSTEM_CONFIG_GET 暴露
给任何已装插件读取)。但真正消费这个配置的 main_routers/community_oauth.py:84
是直接读 os.environ,从未 import 过这个常量——两份定义的兜底逻辑还不一致:

  settings         : os.getenv(..., "neko-servers-desktop-dev").strip() or 默认值
  community_oauth  : 多一层 `raw != "neko-desktop"` 防误配(拒绝复用插件市场 client)

也就是说改 settings 那份不会有任何效果,是个会误导人的埋雷。

方向只能是删 settings 那份,不能反过来让 community_oauth 读它:
scripts/check_module_layering.py 定义 L3=main_routers、L4=plugin,低层不能
import 高层(现有 main_routers/ 下确实零个 `from plugin` import)。

- 删除 NEKO_SERVERS_DESKTOP_CLIENT_ID 常量定义、__all__ 条目、
  PUBLIC_SYSTEM_CONFIG_KEYS 条目(该值是桌面端社区 OAuth 的 client id,
  插件用不到,本就不属于"插件公共配置")
- 把原注释里的防误配意图改挂到 NEKO_AUTH_CLIENT_ID 上,并指明该值归
  main_routers/community_oauth.py 所有
- 新增 test_desktop_client_id_is_owned_here_and_rejects_plugin_market_client,
  把"误配成 neko-desktop 时必须回落"这条意图钉死(原先只有
  `assert "neko-desktop" not in body["auth_url"]` 的间接覆盖)

核实:全仓 grep 该 key 只有 community_oauth 读 env、settings 三处定义/注册、
以及 test_community_oauth 的一处 monkeypatch(env)——settings 那份零消费;
plugin/tests 里对 PUBLIC_SYSTEM_CONFIG_KEYS 的测试是整体 monkeypatch 替换元组,
不钉具体成员,删一个 key 不受影响。

验证:变异验证——删掉 community_oauth 的 `raw != "neko-desktop"` 后新测试立刻变红,
还原后 29 passed;uv run pytest tests/unit -q → 6791 passed, 26 skipped;
uv run pytest plugin/tests -q → 2752 passed, 20 skipped。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

这个 PR 清理了无人消费的桌面端 OAuth client ID 第二真相源,并补强了归属与防误配测试喵。

  • plugin/settings.py 删除 NEKO_SERVERS_DESKTOP_CLIENT_ID 的定义、导出及插件公共配置注册喵。
  • 明确社区桌面端 PKCE client ID 由 main_routers/community_oauth.py 管理喵。
  • 新增测试,覆盖错误复用插件市场 client ID、空值、缺省值和有效生产配置喵。

Confidence Score: 5/5

该 PR 看起来可以安全合并喵。

没有仍需阻止合并的故障喵。

Important Files Changed

Filename Overview
plugin/settings.py 移除未被消费的配置常量及公开注册,现有仓库和关联桌面端仓库均未发现依赖该接口的调用方喵。
tests/unit/test_community_oauth.py 新测试直接验证桌面端 client ID 的默认、误配回退和合法覆盖行为喵。

Reviews (2): Last reviewed commit: "style(test): docstring 改英文以过 DOCSTRING_C..." | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f04148e9-82c8-4529-8dc2-a1f53eda08e9

📥 Commits

Reviewing files that changed from the base of the PR and between 92ed281 and 1cdcd04.

📒 Files selected for processing (2)
  • plugin/settings.py
  • tests/unit/test_community_oauth.py

Comment @coderabbitai help to get the list of available commands.

@wehos
wehos merged commit 6852ea8 into main Jul 28, 2026
12 checks passed
@wehos
wehos deleted the fix/desktop-client-id-single-source branch July 28, 2026 06:01
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