Skip to content

fix(windows): skip unsupported parent-directory fsync - #5002

Merged
huangruiteng merged 1 commit into
loopx-project:mainfrom
Duang777:codex/fix-windows-directory-fsync
Sep 24, 2026
Merged

huangruiteng merged 1 commit into
loopx-project:mainfrom
Duang777:codex/fix-windows-directory-fsync

Conversation

@Duang777

Copy link
Copy Markdown
Contributor

Summary

  • run parent-directory fsync only on POSIX in four atomic writers
  • preserve file fsync and os.replace() on every platform
  • add one parameterized regression for Lark state, decision context, extension projection, and heartbeat automation
  • run the regression in the native Windows Python job

Root cause

Windows rejects opening a directory through os.open(). These writers called it after os.replace(), so the file was committed before the command reported PermissionError.

Closes #4997.

Verification

  • red: tests/test_windows_atomic_writes.py failed all 4 cases before the production fix
  • green: uv run --extra test python -m pytest -q tests/test_windows_atomic_writes.py (4 passed)
  • related suites: 143 passed
  • Ruff: changed Python files passed
  • diff hygiene and Python compile: passed
  • risk-profile and public-boundary premerge checks: passed

The full local premerge did not pass because the fresh worktree lacked the prebuilt Chat bundle and Node parser dependencies. install-local-smoke.py timed out during dependency preparation, codex-cli-packaged-install-smoke.py could not find the Chat bundle, and semantic-vocabulary-drift-smoke.py could not start its TypeScript parser. The changed Python tests and checks passed.

Four atomic writers opened parent directories after os.replace, so Windows reported failure after persisting the file. Guard directory fsync by os.name and add a cross-platform Windows emulation test to the native Windows job.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Copy link
Copy Markdown
Contributor Author

Exact head 401f1b2b6854bf4af840a28176d3b550672567f7 is ready for CI.

Local evidence:

  • regression red before fix: 4/4 Windows directory-open cases failed after the target file was written
  • regression green after fix: 4 passed
  • related Lark, decision-context, presentation, and heartbeat suites: 143 passed
  • Ruff, diff checks, Python compile, risk-profile smokes, and public-boundary scan passed

All GitHub workflows are queued with zero executed steps. Please approve the fork workflows so the native Windows job can validate the new regression.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

动机

公开问题 #4997 记录了 Windows 上 Lark 私有 JSON 写入的真实故障:目标文件已被 os.replace 更新,随后打开父目录做 fsync 却抛 PermissionError,使一次成功替换被报告为失败。此 PR 不改变数据格式或权限模型,目标是让这类本地原子写入在不支持目录 fd 的平台正确返回。

改动思路

保留“临时文件写入 → 文件 fsync → 原子替换”的现有流程,只让替换后的父目录 fsync 在 POSIX 执行。四个写入器仍各自由原来的调用边界管理路径、载荷和失败重试;这与仓库内其他已有的 POSIX guard 一致,没必要为一个平台效果引入新的全局写入器。

具体改动

关键代码讲解

  • loopx/extensions/lark/private_json.py:20 的 write_private_json_atomic:Lark 私有 JSON 的文件 fsync 与替换保持无条件执行,Windows 不再进入父目录 os.open。
  • loopx/capabilities/decision_context/private_state.py:163 的 _write_private_json_atomic:游标持久化应用同一边界,避免写入后报错造成重试歧义。
  • loopx/control_plane/heartbeat/automation_upgrade.py:61 的 _atomic,以及 loopx/extensions/presentation.py:728 的 _atomic_write_projection:覆盖同形的自动化提示与扩展投影写入。
  • 参数化测试用真实临时文件、替换和读回,只模拟 Windows 对父目录打开的拒绝;Windows 工作流也纳入该测试。

对主干的风险

我用同一四入口用例在精确 base 90f21a5 跑出 4 个预期的 PermissionError,在精确 head 401f1b2 跑出 4 个通过;额外 POSIX spy 确认每个入口仍有一次目录打开、两次 fsync。相邻的 Lark、投影、决策游标、自动化测试共 95 个通过,git diff --check 干净,解释器与包源都来自本 worktree。未在本机原生 Windows/NTFS 上独立运行,因此平台特有的 ACL/文件系统行为仍是合并前的剩余验证;这不是我已验证 CI 的声明。没有发现阻断性代码问题。

我的整体评价

APPROVE,针对精确 head 401f1b2b6854bf4af840a28176d3b550672567f7。修复范围与已证实的失败相称,既不吞掉其他写入错误,也没有放弃 POSIX 上原有的目录持久化效果。未来相关的小型重构检查结论:四个写入器保留各自领域所有权更易局部回滚,本 PR 无需再抽象共享层。

English verdict: APPROVE — exact head 401f1b2; no blocking finding. Native Windows validation remains the principal residual risk.

@huangruiteng
huangruiteng merged commit d64c4d3 into loopx-project:main Sep 24, 2026
16 of 24 checks passed
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.

[Bug]: Windows — write_private_json_atomic() raises PermissionError after the write already succeeded (os.open on a directory)

2 participants