Skip to content

Commit 2fa7906

Browse files
wehosclaude
andcommitted
fix(workshop): 同名替换失败回滚旧音频;建目录失败如实上报(Codex P2 ×2)
## 1. 同扩展名替换:manifest 写失败会留下「新音频 + 旧 manifest」 上一版把「删」放到最后之后还剩一个窗口:同扩展名替换时文件名不变, os.replace 会把旧音频原地顶掉,而 manifest 还没写。第 2 步失败的话盘上是新音频配 旧 manifest —— 偏偏文件名没变,_resolve_workshop_voice_reference 认为这对有效, 于是新音频配旧的 prefix / 语言 / display_name / provider,而用户收到的是 500。 我上一轮判断过这个残留并接受了(「两个文件都在,能解析」),判轻了:静默的不一致 比响亮的失败糟得多 —— 用户以为什么都没变,实际参考语音已经被换掉了。 修法:顶上去之前先把旧音频原子挪到 `<tmp>.bak`,任何一步失败就挪回原位(旧 manifest 本来就没动过),成功则在 finally 里删掉备份。回到「要么整对换掉、要么整对不动」。 ## 2. 建目录失败被吞掉,接口照样报 success ensure_workshop_folder_exists 把创建失败(只读盘、权限不足)吞成返回 False,而这里 忽略了返回值。配置确实存下来了,所以 success 仍然是 True —— 但不能因此告诉用户目录 也准备好了,那条路径接下来根本用不了。两件事分开报:新增 folder_ready,为 False 时 附一句 warning。 改响应形状是安全的:全仓库搜不到 POST /api/steam/workshop/config 的任何调用方 (与它此前是死代码、没人发现的事实一致)。 ## 验证 - 去掉备份/回滚 → test_a_same_extension_replace_rolls_back_when_the_manifest_fails 红 - 忽略 ensure 返回值 → test_a_folder_that_cannot_be_created_is_reported 红 - 另加 test_a_successful_replace_leaves_no_backup_behind:成功路径不许留 .tmp/.bak - 全量 tests/unit:8946 passed, 45 skipped;守卫 exit 0;docstring 门 exit 0 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 496d243 commit 2fa7906

4 files changed

Lines changed: 145 additions & 12 deletions

File tree

main_routers/workshop_router/config_files.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ async def save_workshop_config_api(config_data: dict):
6565
# 导入与get_workshop_config相同路径的函数,保持一致性
6666
from utils.workshop_utils import load_workshop_config, save_workshop_config, ensure_workshop_folder_exists
6767

68-
def _apply_config_transaction() -> dict:
68+
def _apply_config_transaction() -> tuple[dict, bool | None]:
6969
with _WORKSHOP_CONFIG_TRANSACTION_LOCK:
7070
# 读也放进锁里:不然两个请求各自读到同一份旧配置、各写各的合并结果,
7171
# 后写的那次会把前一次的字段整份盖掉。
@@ -74,16 +74,25 @@ def _apply_config_transaction() -> dict:
7474
if key in config_data:
7575
merged[key] = config_data[key]
7676
save_workshop_config(merged)
77+
folder_ready: bool | None = None
7778
if merged.get('auto_create_folder', True):
7879
# 优先使用user_mod_folder,如果没有则使用default_workshop_folder
7980
folder_path = merged.get('user_mod_folder') or merged.get('default_workshop_folder')
8081
if folder_path:
81-
ensure_workshop_folder_exists(folder_path)
82-
return merged
83-
84-
workshop_config_data = await asyncio.to_thread(_apply_config_transaction)
85-
86-
return {"success": True, "config": workshop_config_data}
82+
folder_ready = bool(ensure_workshop_folder_exists(folder_path))
83+
return merged, folder_ready
84+
85+
workshop_config_data, folder_ready = await asyncio.to_thread(_apply_config_transaction)
86+
87+
# ensure_workshop_folder_exists 把创建失败(只读盘、权限不足)吞成返回 False。
88+
# 配置确实存下来了,所以 success 仍然是 True —— 但不能因此告诉用户目录也准备
89+
# 好了:那条路径接下来根本用不了。两件事分开报。
90+
response = {"success": True, "config": workshop_config_data}
91+
if folder_ready is not None:
92+
response["folder_ready"] = folder_ready
93+
if not folder_ready:
94+
response["warning"] = "配置已保存,但指定的工坊目录无法创建(路径只读或权限不足)"
95+
return response
8796
except Exception as e:
8897
logger.error(f"保存创意工坊配置失败: {str(e)}")
8998
return {"success": False, "error": str(e)}

main_routers/workshop_router/voice_refs.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,22 +68,40 @@ def _replace_voice_reference(
6868
destroying a reference the user cannot get back.
6969
"""
7070
with voice_reference_lock(content_folder):
71-
# 1) 新音频先落到同目录的 tmp,fsync 之后再 os.replace 上去。
72-
# 失败的话旧的一对纹丝不动 —— 这一步之前不删任何东西。
71+
# 1) 新音频先落到同目录的 tmp,fsync 之后再 os.replace 顶上去。
7372
fd, temp_audio = tempfile.mkstemp(dir=content_folder, suffix='.tmp')
73+
backup_audio = None
7474
try:
7575
with os.fdopen(fd, 'wb') as f:
7676
f.write(audio_bytes)
7777
f.flush()
7878
os.fsync(f.fileno())
79+
80+
# 同扩展名替换时,os.replace 会把旧音频原地顶掉 —— 而 manifest 还没写。
81+
# 万一第 2 步失败,盘上就是「新音频 + 旧 manifest」,偏偏文件名没变,
82+
# _resolve_workshop_voice_reference 会认为这对有效:新音频配旧的
83+
# prefix / 语言 / display_name / provider,而用户收到的是 500。静默的
84+
# 不一致比响亮的失败糟得多,所以先把旧音频原子挪到一边留作回滚。
85+
if os.path.exists(audio_path):
86+
backup_audio = f'{temp_audio}.bak'
87+
os.replace(audio_path, backup_audio)
88+
7989
os.replace(temp_audio, audio_path)
90+
91+
# 2) manifest 也是原子替换。走到这里新的一对才算完整可用。
92+
atomic_write_json(manifest_path, manifest, ensure_ascii=False, indent=2)
8093
except BaseException:
94+
# 回滚到进来时的状态:旧音频挪回原位,旧 manifest 本来就没动过。
95+
if backup_audio is not None and os.path.exists(backup_audio):
96+
with suppress(OSError):
97+
os.replace(backup_audio, audio_path)
8198
with suppress(OSError):
8299
os.remove(temp_audio)
83100
raise
84-
85-
# 2) manifest 也是原子替换。走到这里新的一对已经完整可用。
86-
atomic_write_json(manifest_path, manifest, ensure_ascii=False, indent=2)
101+
finally:
102+
if backup_audio is not None:
103+
with suppress(OSError):
104+
os.remove(backup_audio)
87105

88106
# 3) 最后才清掉「换了扩展名」留下的旧音频(mp3 → wav 这种)。同名的那次
89107
# 已经被上面的 os.replace 顶掉了。删失败只是留个孤儿文件,不影响这对

tests/unit/test_workshop_cloudsave_disabled.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,45 @@ def _ensure(folder):
177177
assert a_ensures == ["ensure:A:auto=True"], (
178178
f"A 的 ensure 读到了别人的配置:{order}"
179179
)
180+
181+
182+
@pytest.mark.asyncio
183+
async def test_a_folder_that_cannot_be_created_is_reported(monkeypatch):
184+
"""Saving a read-only path must not be reported as fully successful.
185+
186+
``ensure_workshop_folder_exists`` swallows the creation failure and returns
187+
False. The config itself did persist, so ``success`` stays True — but the
188+
response has to say the folder is not usable, or the user is told an
189+
unusable workshop path was set up fine.
190+
"""
191+
from main_routers.workshop_router import config_files
192+
from utils import workshop_utils
193+
194+
monkeypatch.setattr(workshop_utils, "load_workshop_config", lambda: {})
195+
monkeypatch.setattr(workshop_utils, "save_workshop_config", lambda cfg: None)
196+
monkeypatch.setattr(workshop_utils, "ensure_workshop_folder_exists", lambda folder: False)
197+
198+
result = await config_files.save_workshop_config_api(
199+
{"user_mod_folder": "R:/read-only", "auto_create_folder": True}
200+
)
201+
202+
assert result["success"] is True, "配置本身确实存下来了"
203+
assert result["folder_ready"] is False
204+
assert "warning" in result
205+
206+
207+
@pytest.mark.asyncio
208+
async def test_a_created_folder_reports_ready(monkeypatch):
209+
from main_routers.workshop_router import config_files
210+
from utils import workshop_utils
211+
212+
monkeypatch.setattr(workshop_utils, "load_workshop_config", lambda: {})
213+
monkeypatch.setattr(workshop_utils, "save_workshop_config", lambda cfg: None)
214+
monkeypatch.setattr(workshop_utils, "ensure_workshop_folder_exists", lambda folder: True)
215+
216+
result = await config_files.save_workshop_config_api(
217+
{"user_mod_folder": "C:/mods", "auto_create_folder": True}
218+
)
219+
220+
assert result["folder_ready"] is True
221+
assert "warning" not in result

tests/unit/test_workshop_voice_refs.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,3 +406,67 @@ def _boom(src, dst):
406406
assert (tmp_path / "voice_sample.mp3").read_bytes() == b"old-audio"
407407
leftovers = [p.name for p in tmp_path.iterdir() if p.name.endswith(".tmp")]
408408
assert leftovers == [], f"失败路径留下了暂存文件:{leftovers}"
409+
410+
411+
def test_a_same_extension_replace_rolls_back_when_the_manifest_fails(tmp_path, monkeypatch):
412+
"""New audio must not survive under the old manifest.
413+
414+
Replacing a .wav with another .wav reuses the filename, so os.replace
415+
overwrites the old audio before the manifest is written. If the manifest
416+
write then fails, resolution still finds a "valid" pair — new audio wearing
417+
the old prefix / language / provider — while the upload answered 500. A
418+
silent mismatch is worse than a loud failure, so the previous audio is
419+
staged aside and restored.
420+
"""
421+
from main_routers.workshop_router import voice_refs
422+
423+
(tmp_path / "voice_sample.wav").write_bytes(b"old-audio")
424+
(tmp_path / WORKSHOP_VOICE_MANIFEST_NAME).write_text(
425+
json.dumps({"version": 1, "reference_audio": "voice_sample.wav", "prefix": "old"}),
426+
encoding="utf-8",
427+
)
428+
429+
def _boom(*args, **kwargs):
430+
raise OSError(28, "No space left on device")
431+
432+
monkeypatch.setattr(voice_refs, "atomic_write_json", _boom)
433+
434+
with pytest.raises(OSError):
435+
voice_refs._replace_voice_reference(
436+
str(tmp_path),
437+
str(tmp_path / "voice_sample.wav"),
438+
b"new-audio",
439+
str(tmp_path / WORKSHOP_VOICE_MANIFEST_NAME),
440+
{"version": 1, "reference_audio": "voice_sample.wav", "prefix": "new"},
441+
)
442+
443+
assert (tmp_path / "voice_sample.wav").read_bytes() == b"old-audio", (
444+
"新音频留在了旧 manifest 底下——同名替换失败后必须回滚"
445+
)
446+
assert _manifest(tmp_path)["prefix"] == "old"
447+
leftovers = sorted(p.name for p in tmp_path.iterdir() if ".tmp" in p.name)
448+
assert leftovers == [], f"失败路径留下了暂存/备份文件:{leftovers}"
449+
450+
451+
def test_a_successful_replace_leaves_no_backup_behind(tmp_path):
452+
"""The staged backup must not outlive a successful swap."""
453+
from main_routers.workshop_router import voice_refs
454+
455+
(tmp_path / "voice_sample.wav").write_bytes(b"old-audio")
456+
(tmp_path / WORKSHOP_VOICE_MANIFEST_NAME).write_text(
457+
json.dumps({"version": 1, "reference_audio": "voice_sample.wav", "prefix": "old"}),
458+
encoding="utf-8",
459+
)
460+
461+
voice_refs._replace_voice_reference(
462+
str(tmp_path),
463+
str(tmp_path / "voice_sample.wav"),
464+
b"new-audio",
465+
str(tmp_path / WORKSHOP_VOICE_MANIFEST_NAME),
466+
{"version": 1, "reference_audio": "voice_sample.wav", "prefix": "new"},
467+
)
468+
469+
assert (tmp_path / "voice_sample.wav").read_bytes() == b"new-audio"
470+
assert _manifest(tmp_path)["prefix"] == "new"
471+
leftovers = sorted(p.name for p in tmp_path.iterdir() if ".tmp" in p.name)
472+
assert leftovers == [], f"成功路径留下了暂存/备份文件:{leftovers}"

0 commit comments

Comments
 (0)