fix(settings): 修复跨窗口设置落盘乱序 - #2573
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
WalkthroughChanges本次变更为全局会话设置加入版本化快照、ETag/If-Match 条件写入、ASR 决策排序与冲突重试，并同步更新 REST 文档及并发测试，喵。 会话设置版本化存储
REST 条件读写
客户端冲突重试
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
Greptile Summary本 PR 为会话设置增加服务端 revision/ETag 条件写入、浏览器冲突合并与跨窗口决策排序，并用线程锁和跨进程文件锁保护共享偏好文件的完整读改写喵
Confidence Score: 4/5此 PR 暂不适合合并，因为延迟到达的 serverMerged 广播仍可撤销另一个窗口已经成功落盘的较新非 ASR 设置喵 接收端只用 _pendingSettingsKeys 保护非 ASR 字段；成功确认后该保护会被清除，而合并广播没有携带逐字段代次，因此更早生成但更晚送达的完整快照仍会被直接应用并可能再次持久化喵 Files Needing Attention: static/app/app-settings.js 喵
|
| Filename | Overview |
|---|---|
| static/app/app-settings.js | 新增 CAS 保存、冲突合并、跨窗口 mutation 代次及恢复广播，但延迟的 serverMerged 快照仍可覆盖已确认的新值喵 |
| main_routers/config_router/preferences.py | 会话设置 API 新增 ETag、If-Match、412 快照和 ASR 决策头处理喵 |
| utils/preferences.py | 偏好文件读改写增加统一线程锁、跨进程锁及版本化会话设置保存喵 |
| tests/unit/test_app_websocket_static.py | 增加 Node 行为回归以覆盖 CAS、ABA、跨窗口合并和恢复广播的 pending 场景喵 |
| tests/unit/test_preferences_concurrency.py | 增加偏好文件并发、版本冲突和进程锁行为测试喵 |
Sequence Diagram
sequenceDiagram
participant A as 窗口 A
participant S as 设置服务
participant B as 窗口 B
A->>S: POST 旧 revision
S-->>A: 412 + 权威快照
A->>A: 写入 serverMerged 快照
B->>S: POST 较新编辑
S-->>B: 成功确认
B->>B: 清除 pending 键
A-->>B: 延迟到达的 serverMerged storage 事件
B->>B: 旧值未被 pending 过滤并被应用
Reviews (13): Last reviewed commit: "fix(settings): keep recovery broadcasts ..." | Re-trigger Greptile
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e68a9f2aa8
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/unit/test_preferences_concurrency.py (1)
204-258: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win顺便补两条契约分支的用例好不好喵？
现在只覆盖了「合法 If-Match → 200/412」。路由里还有两条新分支没人守着：
If-Match格式非法 → 400、X-Conversation-Settings-ASR-Decision非 JSON → 400，以及完全省略If-Match的旧客户端仍能写入。这些正是文档承诺的行为，加三个小断言就能锁住喵～ 需要的话我可以帮你生成喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_preferences_concurrency.py` around lines 204 - 258, Extend test_conversation_settings_route_returns_etag_and_412_snapshot or add focused tests covering the remaining route contracts: malformed If-Match returns 400, malformed X-Conversation-Settings-ASR-Decision JSON returns 400, and a request omitting If-Match still writes successfully for legacy clients. Assert the relevant status codes and preserve existing valid-header behavior.tests/unit/test_app_websocket_static.py (1)
934-962: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win这里正好可以用本文件已有的
_block_after喵
split("function startPeriodicSync()")依赖startPeriodicSync恰好定义在syncSettingsToServer之后；有人重排函数顺序，切片就会膨胀到整个文件尾部，断言随便命中一处也照样通过——正是 Line 20-44 的注释里吐槽过的那种假绿灯喵。改用_block_after用花括号配平取函数体更保险喵～♻️ 建议改法
- sync_fn = settings_source.split( - "async function syncSettingsToServer(options)", 1 - )[1].split("function startPeriodicSync()", 1)[0] + sync_fn = _block_after( + settings_source, "async function syncSettingsToServer(options) {" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_app_websocket_static.py` around lines 934 - 962, Update test_cross_window_settings_posts_use_cas_and_persist_asr_decision_order to extract syncSettingsToServer using the existing _block_after helper with brace-balanced function boundaries instead of splitting at startPeriodicSync. Keep the assertions scoped to that function so reordering adjacent functions cannot produce false positives.utils/preferences.py (1)
474-507: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win字段分类建议从白名单反推，避免以后加键漏配喵
bool_fields手抄了_ALLOWED_CONVERSATION_SETTINGS的大半内容。以后新增一个布尔设置只改白名单、忘了改这里，值会被静默丢弃（校验失败不报错），排查起来很痛苦喵。♻️ 建议改成派生
- bool_fields = { - 'proactiveChatEnabled', 'proactiveVisionEnabled', 'proactiveVisionChatEnabled', - 'proactiveNewsChatEnabled', 'proactiveVideoChatEnabled', 'proactivePersonalChatEnabled', - 'proactiveMusicEnabled', 'proactiveMemeEnabled', 'proactiveMiniGameInviteEnabled', - 'mergeMessagesEnabled', 'focusModeEnabled', 'focusCognitionEnabled', - 'avatarReactionBubbleEnabled', 'slopFilterEnabled', 'subtitleEnabled', - 'noiseReductionEnabled', 'independentAsrEnabled', - 'voiceInputResourceOptimizationEnabled' - } - int_interval_fields = {'proactiveChatInterval', 'proactiveVisionInterval'} - string_fields = {'userLanguage'} - int_limit_fields = {'textGuardMaxLength'} + int_interval_fields = {'proactiveChatInterval', 'proactiveVisionInterval'} + string_fields = {'userLanguage'} + int_limit_fields = {'textGuardMaxLength'} + bool_fields = _ALLOWED_CONVERSATION_SETTINGS - ( + int_interval_fields | string_fields | int_limit_fields + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/preferences.py` around lines 474 - 507, 调整_validate_conversation_settings中的字段分类，使bool_fields由_ALLOWED_CONVERSATION_SETTINGS扣除int_interval_fields、string_fields和int_limit_fields等已明确类型的字段派生，而不是手工维护布尔键列表；保留现有各类型校验规则，确保新增白名单设置在未被其他类型集合声明时按布尔值校验。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main_routers/config_router/preferences.py`:
- Around line 232-302: Update the initial non-dict request-body branch in
save_conversation_settings to return a 400 JSONResponse, matching the existing
malformed If-Match and ASR-decision header branches, while preserving the
current error payload.
In `@static/app/app-settings.js`:
- Around line 148-166: Update _mergeConversationSettingsConflict to apply merged
shared settings through applySharedRuntimeSettings instead of only assigning
server values to S, so corresponding window.* mirrors stay synchronized;
preserve dirty-key and independentAsrEnabled conflict rules, and continue
updating _settingsBaseline for accepted server values.
---
Nitpick comments:
In `@tests/unit/test_app_websocket_static.py`:
- Around line 934-962: Update
test_cross_window_settings_posts_use_cas_and_persist_asr_decision_order to
extract syncSettingsToServer using the existing _block_after helper with
brace-balanced function boundaries instead of splitting at startPeriodicSync.
Keep the assertions scoped to that function so reordering adjacent functions
cannot produce false positives.
In `@tests/unit/test_preferences_concurrency.py`:
- Around line 204-258: Extend
test_conversation_settings_route_returns_etag_and_412_snapshot or add focused
tests covering the remaining route contracts: malformed If-Match returns 400,
malformed X-Conversation-Settings-ASR-Decision JSON returns 400, and a request
omitting If-Match still writes successfully for legacy clients. Assert the
relevant status codes and preserve existing valid-header behavior.
In `@utils/preferences.py`:
- Around line 474-507:
调整_validate_conversation_settings中的字段分类，使bool_fields由_ALLOWED_CONVERSATION_SETTINGS扣除int_interval_fields、string_fields和int_limit_fields等已明确类型的字段派生，而不是手工维护布尔键列表；保留现有各类型校验规则，确保新增白名单设置在未被其他类型集合声明时按布尔值校验。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a711cad6-d479-4edd-8469-55546c8d1efd
📒 Files selected for processing (8)
docs/api/rest/config.mddocs/ja/api/rest/config.mddocs/zh-CN/api/rest/config.mdmain_routers/config_router/preferences.pystatic/app/app-settings.jstests/unit/test_app_websocket_static.pytests/unit/test_preferences_concurrency.pyutils/preferences.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21c53cac48
ℹ️ 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".
|
Handled the three CodeRabbit nitpicks in 70cc9e0 as well: added malformed If-Match / malformed ASR decision / legacy no-If-Match contract coverage; switched the syncSettingsToServer source test to brace-balanced _block_after extraction; and derived boolean conversation-setting fields from the allowlist minus explicitly typed non-boolean sets. Full related suite: 73 passed; Ruff passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df9323c4ec
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c79b4d1fde
ℹ️ 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".
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
N.E.K.O/static/app/app-settings.js
Line 1526 in f646434
Adopt authoritative storage decision tuples before re-sharing
When one window merges a server ASR winner, saveSettings({serverMerged: true}) broadcasts its asrDecision with an empty changedKeys. A sibling that previously toggled ASR applies this authoritative value but skips this block, so _lastAsrDecision remains stale; because _dirtySettingsKeys is monotone, the sibling's next unrelated save then declares ASR explicit and _noteAsrDecision assigns the accepted value a fresh ID. That incidental save can outrank and overwrite a genuine toggle that has not reached the sibling yet. Record an applied authoritative meta.asrDecision even when the incoming snapshot is not marked as an explicit cross-window toggle.
ℹ️ 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".
|
已处理 Codex review #4817112402 中的 Adopt authoritative storage decision tuples before re-sharing（b5520e208）。判断正确：服务端合并广播虽然 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5520e208c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17e964b43c
ℹ️ 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".
| } | ||
| } | ||
| _noteCrossWindowMutations(incoming, meta ? meta.changedKeys : null); | ||
| const changed = applySharedRuntimeSettings(incoming); |
There was a problem hiding this comment.
当窗口 B 的非 ASR 编辑已由 POST 成功确认并从 _pendingSettingsKeys 清除，而窗口 A 更早生成的 serverMerged 广播随后才送达时，监听器不会过滤该字段，并在这里直接应用广播中的旧值，导致窗口 B 的运行时与 localStorage 设置回退，后续同步还会把旧值重新写回服务端喵。
1 participant
改动概述 / Summary
Fixes #2538。
/api/config/conversation-settings增加 revision + ETag/If-Match 条件写入；冲突返回412和当前权威快照。(writeId, writerId, value)ASR 决策全序，并通过请求头持久化到服务端，避免较旧窗口请求最后完成时覆盖较新选择。portalocker跨进程锁覆盖user_preferences.json的完整 read-modify-write，同时保护模型偏好、主动搭话和内部设置写入，避免不同字段互相丢失。回归报告 / Regression Report
不适用：未修改
app/、main_logic/或memory/。不拆分理由 / Why Not Split
不适用：计入上限的文件数不超过 20；服务端 CAS、浏览器重试和共享存储锁共同构成同一个原子修复，拆开会留下竞态窗口。
测试 / Testing
uv run ruff check utils/preferences.py main_routers/config_router/preferences.py tests/unit/test_preferences_concurrency.pyuv run pytest tests/unit/test_preferences_concurrency.py tests/unit/test_app_websocket_static.py tests/unit/test_preferences_strict_read.py tests/unit/test_cloudsave_config_manager.py tests/unit/test_cloudsave_runtime.py tests/unit/test_proactive_interval_20s_rollback.py tests/unit/test_proactive_service_boundary.py tests/unit/test_core_independent_asr.py -qgit diff --check origin/main...HEADSummary by CodeRabbit
新功能
文档