fix(windows): skip POSIX-only fchmod in atomic private writers - #5008
Conversation
The native Windows lane fails in `test_atomic_writers_skip_unsupported_windows_directory_fsync[lark-private-json]` because `loopx/extensions/lark/private_json.py` calls `os.fchmod`, which does not exist on Windows. The AttributeError then leaves the mkstemp descriptor open, so the cleanup unlink fails too and masks the original error with `PermissionError: [WinError 32]`. Guard the POSIX-only permission call in the four atomic writers that still used it unguarded (lark private JSON, botmux binding, benchmark native isolation copy, decision-context spool), matching the existing `hasattr(os, "fchmod")` pattern already used by codex_cli, agent_turn_recall, reward_memory and external_connector_runtime. mkstemp/os.open already created each file with the requested mode, so the POSIX behavior is unchanged. The Windows shim in the atomic-writes test now reports `fchmod` as missing the way native Windows does, and the parametrization covers the botmux binding and native isolation writers. Without the guards those two entries fail with `AttributeError: fchmod` on the POSIX lane, and the lark entry reproduces the native CI failure; with the guards all six entries pass. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head: f2a18a9931f927cb21fd2b048d641a2b8f7162a3
动机
main 的 windows-powershell 这条 lane 现在是红的。失败 job(run 36004507686,head d64c4d377)在 test_atomic_writers_skip_unsupported_windows_directory_fsync[lark-private-json] 上报:
loopx\extensions\lark\private_json.py:24: AttributeError: module 'os' has no attribute 'fchmod'
...
PermissionError: [WinError 32] ... '.private_json.json.nso4cg2w.tmp'
os.fchmod 是 POSIX-only。它在 os.fdopen 接管 mkstemp 描述符之前就抛错,于是描述符一直开着,finally 里的 temporary.unlink() 在 Windows 上又因文件被占用失败,把原始错误盖掉——所以日志看起来像「临时文件删不掉」,实际根因是权限调用不该在 Windows 上执行。PR #5002 只把目录 fsync 收敛到 POSIX,漏掉了这第二个 POSIX-only 调用;同样的写法在另外三处 atomic writer 里也存在。
改动思路
修复方式是复用仓库里已经确立的平台能力判据,而不是新造抽象:if hasattr(os, "fchmod"): 这一行在 codex_cli、agent_turn_recall、reward_memory、external_connector_runtime 里已经是既有做法。mkstemp 与 os.open(..., 0o600) 本身已经按请求的权限创建文件,所以在有权限位的平台上行为不变;在 Windows 上不再抛错,而是用平台默认保护写文件,并且描述符照常交给 os.fdopen,finally 清理恢复可用。
比修 CI 点名的那一处更重要的是:原来的 Windows shim 只模拟「目录 fsync 不支持」,却把 fchmod 转发给宿主 os,所以 POSIX lane 永远看不到这类缺陷——这正是它能进 main 的原因。因此本 PR 让 shim 像真实 Windows 一样把 fchmod 也报成缺失,并把同样有缺陷的两个 writer 纳入参数化,使这条约束以后在 POSIX lane 上就能拦住。
具体改动
四个生产文件各加一处守卫(每个 3+/1-,含一行说明):loopx/extensions/lark/private_json.py:25、loopx/control_plane/goals/botmux_runtime.py:167、loopx/capabilities/benchmark_toolkit/native_codex_isolation.py:300、loopx/capabilities/decision_context/capture.py:45;tests/test_windows_atomic_writes.py(+26/-1)更新 shim 并新增 botmux-binding、native-isolation 两个参数化条目。
关键代码讲解
_WindowsOs.__getattr__(tests/test_windows_atomic_writes.py:30)的取值集合从 {"O_DIRECTORY"} 扩到 {"O_DIRECTORY", "fchmod"}。这是整份改动里最关键的一行:它让「在 POSIX 上模拟 Windows」不再把只存在于 POSIX 的属性泄漏出去,从而使参数化测试真正覆盖 Windows 表面。
private_json.write_private_json_atomic(loopx/extensions/lark/private_json.py:25)是 CI 实际命中的那处;守卫放在 os.fdopen 之前,因此既避免了 AttributeError,也消除了「描述符未交出去 → unlink 失败」的连带错误。botmux_runtime._write_private_json_atomic(:167)与它结构相同,属于同一类私有 JSON 写入。
native_codex_isolation._atomic_write_text(:300)的守卫还顺带避免了在 Windows 上求值 path.stat().st_mode & 0o777——那里的权限位本身在 Windows 上没有意义。capture._open_spool(:45)则在 os.open(..., 0o600) 之后做同样的条件调用,并在随后立即 os.close,不依赖 finally 清理。
对主干的风险
最需要防的是「跳过权限调用等于放松保护」。具体场景是 Windows 上写私人绑定或决策 spool:如果守卫让文件变成其他本地用户可读,那比报错更糟。实际语义是——Windows 没有 POSIX 权限位,也没有 fchmod,mkstemp/os.open 已经按创建者默认保护建好文件;守卫保留的是该平台能提供的最强保护,而不是降低它。POSIX 侧 os.fchmod 仍然原样执行,模式参数没有变化。
第二类风险是范围外扩:我顺手把另外三处同类未守卫调用一起改了。它们的调用路径是 Lark 扩展、botmux Goal binding、benchmark 原生隔离与 decision-context capture——同一缺陷、同一行修复、同一测法,但其中 botmux 与 native-isolation 的覆盖是本次新增条目的,decision-context spool 因为产物是 SQLite 而没有进这个参数化测试(只在既有 36 个测试里回归)。这一条是本次结论里最明确的覆盖边界。
第三是证据边界:真正的原生 Windows 执行在 CI,本机是 macOS,本轮只在 POSIX 上用「隐藏 fchmod 的 shim」复现并验证;按该 Goal 解析出的 wait_for_ci=false,本评审没有等远端 CI。因此 native lane 记为合并后 readback,而不是已通过。
我的整体评价
这是对 main 现有红灯的最小正确修复:根因是平台能力判断缺失,修法是仓库已有的同一模式,且补上了让这类缺陷无法再溜过 POSIX lane 的测试条件。
独立验证记录:tests/test_windows_atomic_writes.py 6 passed;把守卫去掉后(shim 忠实隐藏 fchmod),lark-private-json 复现原生 CI 的 AttributeError: fchmod,新增的 botmux-binding、native-isolation 也各自失败(2 failed / 4 passed),恢复守卫后 6 passed——即这条回归确实有牙。聚焦选择 test_windows_atomic_writes + test_lark_goal_channel_lifecycle + test_project_registry + test_periodic_report_pending_intent 122 passed,test_decision_context_capture{,_recovery} 36 passed;loopx canary premerge --from-git-diff 选中检查全过(semantic-vocabulary-drift-smoke 在全新 worktree 首次运行缺 node_modules,链接共享依赖树后通过,属环境而非本次 diff)。
建议合并,并在合并后回收 windows-powershell 结果确认原生路径。
English verdict: APPROVE - The change replaces an unguarded POSIX-only syscall with the repository's established capability check in all four affected atomic writers, keeps POSIX permission behavior unchanged, and makes the Windows test shim faithful so the POSIX lane now fails if any of these writers regress; mutation runs reproduce the native CI AttributeError and the guarded head passes 6/6 plus the focused suites.
Problem
The native Windows lane (
windows-powershell) is red onmain. The failing job(
36004507686, headd64c4d377) reports:os.fchmodis POSIX-only. On Windows the call raises, and because it runs beforeos.fdopentakes ownership of themkstempdescriptor, the descriptor stays open; thefinally: temporary.unlink(missing_ok=True)cleanup then fails withWinError 32and masksthe original error. PR #5002 gated the directory
fsyncto POSIX but left this secondPOSIX-only call unguarded.
Change
unguarded:
loopx/extensions/lark/private_json.py,loopx/control_plane/goals/botmux_runtime.py,loopx/capabilities/benchmark_toolkit/native_codex_isolation.pyandloopx/capabilities/decision_context/capture.py.(
hasattr(os, "fchmod"), as incodex_cli,agent_turn_recall,reward_memoryandexternal_connector_runtime).mkstemp/os.openalready create each file with the requestedmode, so POSIX behavior is unchanged and no descriptor is left open on Windows.
tests/test_windows_atomic_writes.pyfaithful: it now reportsfchmodas missing the way native Windows does, and the parametrization also covers thebotmux binding and benchmark native-isolation writers.
Why the test change matters
The previous shim only simulated the unsupported directory
fsync; it forwardedfchmodto the host
os, so the POSIX lane could never observe this failure class. That is why aWindows-only defect reached
mainwith a green POSIX run.Validation
uv run --extra test python -m pytest tests/test_windows_atomic_writes.py -q— 6 passed.lark-private-jsonentry reproduces the native CI failure (AttributeError: fchmod) and thenew
botmux-binding/native-isolationentries fail the same way (2 failed, 4 passed);with the guards all six pass.
uv run --extra test python -m pytest tests/test_windows_atomic_writes.py tests/extensions/test_lark_goal_channel_lifecycle.py tests/cli_commands/test_project_registry.py tests/capabilities/test_periodic_report_pending_intent.py -q— 122 passed.uv run --extra test python -m pytest tests/capabilities/test_decision_context_capture.py tests/capabilities/test_decision_context_capture_recovery.py -q— 36 passed.loopx canary premerge --from-git-diff --git-diff-base origin/main— all selected checkspassed;
examples/semantic-vocabulary-drift-smoke.pyneeds anode_moduleslink in a freshworktree and passes once the shared dependency tree is linked (environmental, not a diff issue).
Boundary
Behavior change is limited to Windows, where the previous code raised instead of writing the
file. POSIX permission handling is unchanged, and no state, protocol or CLI contract changes.