Skip to content

Commit 8505cea

Browse files
wehosclaude
andcommitted
review: 前端 409 也按 error_code 判别,GET 读后补一次身份校验
Codex 两条,核实后都成立。 1. 我上一轮只在后端区分了 409 来源,前端 `_saveCharacterLanguagePreference` 仍是无条件 `response.status === 409`。主服务端的 main_storage_limited_mode_guard 对未白名单路径(本路由不在白名单)与 MaintenanceModeError 同样返 409,此时 前端会宣告「被更新的偏好取代」并去重新水合,水合再失败就既不回滚也不报错, 留下一个未持久化的选中值。改为只认 error_code=language_preference_superseded, 其余 409 落到原有失败路径(回滚 + 报错)。 2. GET 去掉锁之后,`_load_existing_character` 通过到 memory server 读返回之间, 角色可能被别的窗口删除/改名,端点仍会为已不存在的名字返回 200;进行中的卡片 管理器水合可能在删除清理之后用这个陈旧响应重填 per-name 语言缓存,被后续复用 的同名角色继承。读后补一次身份校验。 守卫:新增 tests/unit/test_language_preference_conflict_frontend.py,用 node harness 跑真实的 _saveCharacterLanguagePreference 源码(分支行为静态断言抓不到), 覆盖 superseded / 存储受限 / 维护围栏 / 无 code 四种 409 与成功路径,并钉住三处 error_code 字面量一致。后端补 test_language_preference_get_revalidates_after_the_unlocked_read。两条均做变异验证。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 40f4a3d commit 8505cea

4 files changed

Lines changed: 213 additions & 1 deletion

File tree

main_routers/characters_router/language_preference.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,12 @@ async def get_character_language_preference(name: str):
382382
# no longer leave an empty old-name directory behind.
383383
await _load_existing_character(name)
384384
payload = await _request_memory_prompt_locale("GET", name)
385+
# Dropping the lock also dropped the guarantee that the character still
386+
# exists once the read returns. Without a second check this would answer
387+
# 200 for a name deleted mid-read, and an in-flight card-manager
388+
# hydration could repopulate that name's local language cache after the
389+
# deletion cleanup -- which a later reuse of the same name would inherit.
390+
await _load_existing_character(name)
385391
ui_language = await aload_ui_language_override()
386392
payload["effective_language"] = (
387393
payload.get("language")

static/js/character_card_manager/card-form-and-actions.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,15 @@ async function _saveCharacterLanguagePreference(name, select, selectUi) {
219219
// A cross-window event or a newer local request may have superseded this
220220
// response while it was in flight. Never roll back or cache stale data.
221221
if (select.dataset.languageSaveId !== saveId || select.value !== language) return;
222-
if (response.status === 409) {
222+
// Only this endpoint's causal-order conflict means "re-read the state".
223+
// Both servers also answer 409 for storage-limited startup and for the
224+
// cloudsave maintenance fence; those persisted nothing, so they must
225+
// fall through to the failure path below (roll back + report) instead
226+
// of leaving an unsaved selection on screen.
227+
if (
228+
response.status === 409
229+
&& payload.error_code === 'language_preference_superseded'
230+
) {
223231
// Designed race, not a failure: another window persisted a newer
224232
// preference. Rolling back to this window's previous value would
225233
// display a locale that is already stale, so re-read durable state.
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
"""The card manager must only treat this endpoint's own 409 as superseded.
2+
3+
Both servers answer 409 for storage-limited startup and for the cloudsave
4+
maintenance fence too. Those persisted nothing, so announcing "a newer
5+
preference won" and re-hydrating would leave an unsaved selection on screen.
6+
7+
Driven through the real ``_saveCharacterLanguagePreference`` source rather than
8+
asserted statically: the distinction lives in a branch, and a static check
9+
cannot tell which path a given payload actually takes.
10+
"""
11+
12+
import json
13+
import re
14+
import shutil
15+
import textwrap
16+
from pathlib import Path
17+
18+
import pytest
19+
20+
from tests.node_harness import run_node_script
21+
22+
23+
PROJECT_ROOT = Path(__file__).resolve().parents[2]
24+
SOURCE = (
25+
PROJECT_ROOT
26+
/ "static"
27+
/ "js"
28+
/ "character_card_manager"
29+
/ "card-form-and-actions.js"
30+
)
31+
32+
33+
@pytest.fixture(scope="module")
34+
def node_path():
35+
executable = shutil.which("node")
36+
if not executable:
37+
pytest.skip("node is required for browser runtime harnesses")
38+
return executable
39+
40+
41+
def _extract_function(name: str) -> str:
42+
source = SOURCE.read_text(encoding="utf-8")
43+
start = source.find(f"async function {name}(")
44+
assert start >= 0, f"{name} 已改名,请同步更新测试"
45+
depth = 0
46+
for index in range(start, len(source)):
47+
if source[index] == "{":
48+
depth += 1
49+
elif source[index] == "}":
50+
depth -= 1
51+
if depth == 0:
52+
return source[start : index + 1]
53+
raise AssertionError(f"{name} 括号不平衡")
54+
55+
56+
def _harness(status: int, payload: dict) -> str:
57+
return textwrap.dedent(
58+
"""
59+
const calls = [];
60+
let hydrated = 0;
61+
62+
function _characterLanguageT(_key, fallback) { return fallback; }
63+
function showMessage(text, level) { calls.push(['message', level]); }
64+
async function showAlert(text) { calls.push(['alert', String(text)]); }
65+
async function _hydrateCharacterLanguagePreference() { hydrated += 1; }
66+
async function _characterLanguageMutationFetch() {
67+
return {
68+
status: __STATUS__,
69+
ok: __STATUS__ >= 200 && __STATUS__ < 300,
70+
async json() { return __PAYLOAD__; },
71+
};
72+
}
73+
function _cacheCharacterLanguagePreference() { calls.push(['cache']); }
74+
75+
__FUNCTION__
76+
77+
const select = {
78+
value: 'ja',
79+
dataset: { previousValue: 'en' },
80+
disabled: false,
81+
};
82+
83+
_saveCharacterLanguagePreference('Mimi', select, null).then(() => {
84+
console.log(JSON.stringify({
85+
hydrated,
86+
value: select.value,
87+
previousValue: select.dataset.previousValue,
88+
calls,
89+
}));
90+
});
91+
"""
92+
).replace("__FUNCTION__", _extract_function("_saveCharacterLanguagePreference")) \
93+
.replace("__STATUS__", str(status)) \
94+
.replace("__PAYLOAD__", json.dumps(payload))
95+
96+
97+
def _run(node_path: str, status: int, payload: dict) -> dict:
98+
result = run_node_script(
99+
node_path, _harness(status, payload), capture_output=True, timeout=30,
100+
)
101+
assert result.returncode == 0, result.stderr
102+
return json.loads(result.stdout.strip().splitlines()[-1])
103+
104+
105+
def test_superseded_conflict_rehydrates_without_rolling_back(node_path):
106+
outcome = _run(
107+
node_path,
108+
409,
109+
{"success": False, "error_code": "language_preference_superseded"},
110+
)
111+
112+
assert outcome["hydrated"] == 1
113+
# The stale local value must not be restored; hydration owns the control.
114+
assert outcome["value"] == "ja"
115+
assert not any(call[0] == "alert" for call in outcome["calls"]), (
116+
"被取代不是保存失败,不该弹错误"
117+
)
118+
119+
120+
@pytest.mark.parametrize(
121+
"payload",
122+
[
123+
{"ok": False, "error_code": "storage_startup_blocked", "limited_mode": True},
124+
{"success": False, "code": "cloudsave_maintenance", "retryable": True},
125+
{"success": False},
126+
],
127+
)
128+
def test_unrelated_409_rolls_back_and_reports_failure(node_path, payload):
129+
outcome = _run(node_path, 409, payload)
130+
131+
assert outcome["hydrated"] == 0, "非 superseded 的 409 不该触发重新水合"
132+
assert outcome["value"] == "en", "保存失败必须回滚到先前的值"
133+
assert any(call[0] == "alert" for call in outcome["calls"]), "必须报告保存失败"
134+
135+
136+
def test_successful_save_still_caches_and_reports(node_path):
137+
outcome = _run(node_path, 200, {"success": True, "language": "ja"})
138+
139+
assert outcome["hydrated"] == 0
140+
assert outcome["value"] == "ja"
141+
assert outcome["previousValue"] == "ja"
142+
assert any(call[0] == "cache" for call in outcome["calls"])
143+
144+
145+
def test_frontend_conflict_code_matches_the_backend_constant():
146+
"""Pin the two ends of the wire contract to the same literal."""
147+
frontend = SOURCE.read_text(encoding="utf-8")
148+
backend = (
149+
PROJECT_ROOT / "main_routers" / "characters_router" / "language_preference.py"
150+
).read_text(encoding="utf-8")
151+
memory_server = (
152+
PROJECT_ROOT / "app" / "memory_server" / "routes.py"
153+
).read_text(encoding="utf-8")
154+
155+
code = "language_preference_superseded"
156+
assert re.search(rf"error_code\s*===\s*'{code}'", frontend)
157+
assert f'"{code}"' in backend
158+
assert f'"{code}"' in memory_server

tests/unit/test_language_preference_lock_scope.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,46 @@ async def serialized(data):
368368
assert observed["payload"]["character_card_name"] == "Mimi"
369369

370370

371+
async def test_language_preference_get_revalidates_after_the_unlocked_read(monkeypatch):
372+
"""Dropping the lock dropped the "still exists when we answer" guarantee.
373+
374+
Without a second check the endpoint answers 200 for a name deleted while the
375+
memory-server read was in flight, and an in-flight card-manager hydration
376+
could repopulate that name's local cache after the deletion cleanup.
377+
"""
378+
state = {"exists": True}
379+
loads = []
380+
381+
async def load_character(name):
382+
loads.append(name)
383+
if not state["exists"]:
384+
raise LookupError("角色不存在")
385+
return SimpleNamespace(memory_dir="unused"), {"猫娘": {name: {}}}
386+
387+
async def request_locale(_method, _name, *, language=None):
388+
# The character is deleted while this read is awaiting.
389+
state["exists"] = False
390+
return {"success": True, "language": "ja"}
391+
392+
monkeypatch.setattr(preference_router, "_load_existing_character", load_character)
393+
monkeypatch.setattr(preference_router, "_request_memory_prompt_locale", request_locale)
394+
monkeypatch.setattr(
395+
preference_router, "aload_ui_language_override", lambda: _async_value("en"),
396+
)
397+
398+
response = await preference_router.get_character_language_preference("Mimi")
399+
400+
assert getattr(response, "status_code", 200) == 404
401+
assert loads == ["Mimi", "Mimi"], "读前读后都要校验角色身份"
402+
403+
404+
def _async_value(value):
405+
async def _inner():
406+
return value
407+
408+
return _inner()
409+
410+
371411
def test_prompt_locale_read_does_not_create_the_character_directory(tmp_path, monkeypatch):
372412
import utils.config_manager as config_manager_module
373413

0 commit comments

Comments
 (0)