Repository navigation
fix(checkpoint): flush captured members through update handles on Windows - #6566
JasonBuildAI wants to merge 1 commit into
Conversation
JasonBuildAI
left a comment
There was a problem hiding this comment.
Independent review — merge-ready, no blocking findings
Independent review of head e6d3b4728050328ebc1589db6d58d55eb617d7bb. This review was drafted with AI assistance at the operator's request, under their direction and review; every claim below was re-verified against this exact head on native Windows rather than accepted from the PR body.
English verdict: merge-ready — no blocking findings. Merge is intentionally left to the maintainer (author-owned PR, no self-merge), pending the remaining CI lanes.
Verified on the exact head
- Root cause confirmed.
fs.open(copiedPath, "r")+fsyncfailsEPERM -4048on Windows for every copied file becauseFlushFileBuffersrequires a write-capable handle. Focused suite on base3bc74f931: 5 passed / 8 failed (EPERM -4048atcopyComponent); on this head: 14 passed / 0 failed. - Fix semantics correct on both platforms. Writable copies flush through an
r+handle. OnEACCES/EPERMthe copy's mode is read vialstat, grantedmode | 0o600, flushed viar+, then restored withchmod(path, mode)infinally— so the #6415 boundary (POSIX non-root read-only participant copies must not fail) still holds and no copy is ever witnessed in a granted mode. Non-permission errors propagate before any chmod; the durability flush is never silently skipped. - Adversarial probe (independently re-run). Real
runtime.checkpoint_artifactscapture + materialize + verify with a read-only participant (0o444) and a standalone0o400file:capture integrity: true,source stability: true;artifact_witnessmodes equalsource_witnessmodes (444/666preserved);- materialize
integrity_verified: true, restored modes and bytes match; - verify
integrity_verified: true; - missing-source probe rejects
ENOENT(fail-fast intact).
- Regression test. The new read-only participant case fails on base and passes on head; the added symlink assertion uses the established
process.platform !== "win32"convention (same as merged #6356) and is test-only. - Sibling path unchanged. The SQLite snapshot destination in
sqlite_backup.tsis always created0o600, so its plainr+open remains the correct narrow contract; no shared-helper drift was introduced.
Non-blocking observations
- A hybrid
r+→r+sync → chmod ordering is an alternative; the current order is fine and simpler to reason about. - The checkpoint suite is not part of the
windows-powershellCI lane, which is why this Windows-only regression escaped CI; adding it there is a reasonable follow-up, not a requirement for this PR. - Full TS control-plane suite on the rebased head: 4699 passed / 2 failed / 75 skipped; both failures are host/unrelated (
zcode_app_server.test.tsprocess_cleanup_unconfirmedalso fails on clean base3bc74f931;succession_read_conformance.tspasses 1/1 in isolation).
CI at review time
Sign-off, changes, dependency-review, postgresql-authority, node-minimum-compatibility, chat-bundle passed; build, typescript-core (3 shards), windows-powershell, stage2c, and browser lanes were pending.
…oopx-project#6566) Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
…dows On native Windows every runtime.checkpoint_artifacts capture and materialize aborts with EPERM -4048 fsync: copyComponent flushes each copied member through a read-only handle (open(destination, "r") + sync()), and Windows FlushFileBuffers requires update access. loopx-project#6415 made the flush read-only so an ordinary non-root POSIX controller can capture a read-only participant's 0o444 copy, and the flush itself is deliberate (loopx-project#6258). Keep the flush on every platform: open the staged copy for update, and when the copy is read-only (POSIX 0o444 mode or the Windows read-only attribute) grant owner access only for the sync, then restore the reviewed mode before the copy is witnessed or published. The checkpoint artifact suite gains a read-only participant case and the established Windows symlink host-privilege guard; 14 of 14 tests pass on native Windows (was 5 of 13). Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
e6d3b47 to
5fe5470
Compare
Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
|
Rebase note — head updated to The The COMMENTED review above remains attached to the previous head |
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; model=gpt-6.1-sol; provider=OpenAI; runtime_reported; reasoning_effort=max
未发现阻塞问题;APPROVE。本机是普通用户 macOS,Windows 验证的边界在下文单独说明。
动机
在 Windows 上保存或恢复研究检查点的使用者,以及捕获只读研究文件的普通 POSIX 用户。旧实现用只读句柄刷新刚复制的文件,Windows 会拒绝这一步,导致捕获或恢复中断;直接改成可写句柄又会拒绝只读文件。本提案在私有暂存副本上临时开放所有者访问,刷新后还原权限,再校验和发布。同一次捕获与恢复应保留文件字节和原权限,刷新失败则不发布检查点,且不修改原文件或激活恢复出的历史。本次只修复文件刷新机制,不新增自动捕获、执行权限、研究效果声明或前端恢复入口。
改动思路
入口仍是 runtime.checkpoint_artifacts,由显式 research-checkpoint CLI 和已有 Codex/research Host 使用。capture 先见证源文件,再复制到私有暂存目录、刷新、比对字节及模式、验证 manifest 后原子发布;materialize 验证原单元,再走同一复制刷新路径并验证结果。刷新属于既有 TypeScript 检查点 IO owner 的完整性要求;临时改权仅作用于新建私有暂存副本,失败关闭,独立 SQLite 快照仍保留自己的可写目的文件契约。本 PR 限于 capture/materialize 的复制后刷新和权限恢复,沿用原有校验及原子发布,不扩展到历史激活、跨宿主权限或新的捕获策略。
可写副本直接以 r+ 刷新;只有 EACCES/EPERM 才读取副本权限、添加所有者读写、重试,并在 finally 还原。其他错误原样失败。已有 SQLite snapshot owner 已把目的文件创建为0600,它与任意只读参与文件的语义不同,强行合并为通用跨 owner 刷新框架没有收益。这里抽出的两个局部函数让重试和句柄关闭更容易检查,属于有界的后续可维护性改进。
具体改动
依据 issue #6562 的捕获/恢复缺陷及该问题记载的基线 81ff72b64,并对照当前实际 merge base 32c882f 的 docs/reference/research-checkpoint.md。文档将此入口明确限定为显式 CLI/Host 阶段;本次不要求它顺带完成原本尚未交付的前端恢复旅程。微软 FlushFileBuffers 接口说明 要求句柄具有写访问权,支持此处改用 update handle。
- CHECKPOINT-FLUSH:implemented。 保留每个复制文件的 sync,采用可更新句柄,没有按 Windows 跳过 durability。
- CHECKPOINT-READONLY:implemented。 0444 和0400 的普通用户文件均可 capture、materialize、verify,字节及原模式保留,原文件未改动。
- CHECKPOINT-FAIL-CLOSED:implemented。 非权限 EIO 不触发 chmod;只读 fallback 后 sync 失败仍尝试还原;grant 或 restore 失败不发布单元,暂存清理且源文件不变。
- CHECKPOINT-INERT:implemented。 CLI/Host 及 runtime readback 保留 execution_authority_granted=false、activation_performed=false;恢复历史不赋予当前执行资格。
关键代码讲解
syncCopiedFile,第117行:r+ 打开已复制文件,sync 后在 finally 关闭句柄;文件刷新失败不会被当成成功。flushCopiedFile,第128行:仅 EACCES/EPERM 进入临时改权路径,保存原 mode,并在重试后恢复;操作对象是新建0700目录内的副本。copyComponent,第139行:继续拒绝 symlink,SQLite 用已有 snapshot owner,普通文件 copyFile 后调用刷新 helper。capture/materialize 的后续 witness 与发布顺序不变。
测试增加只读参与文件的捕获和恢复断言。Windows 分支保留 junction 源和目的路径拒绝,文件 symlink 子断言因系统创建权限只在非 Windows 执行;这项覆盖限制公开可见。完整差异为两文件 +47/-8,没有状态词汇、CLI 参数或持久化格式变化。
对主干的风险
独立执行:TS checkpoint_artifacts 与 sqlite_backup 当前 17 passed,实际基线 16 passed;Python 三份 checkpoint 文件首轮 56 passed、12 skipped。随后显式使用真实 Codex 二进制补跑原生 Host 参数组,13 passed、47 deselected,覆盖此前12个 skip(其中一个非原生对照重复)。这些原生进程连接本地确定性推理服务,真实保存/读取 SQLite、捕获并在新环境恢复上下文,没有付费模型或真实研究效用证明。另有八类真实文件 IO/失败注入探针、完整 TypeScript typecheck,以及 native canary 三项直接检查、八项选择检查通过,含 advisory 后的完整语义检查。
最强反例是:只读文件能够捕获,但刷新或还原权限失败后仍发布错误模式的单元。实测0444/0400各连续恢复两次并校验;open EIO、fallback sync EIO、临时改权失败、还原失败均无发布、无残留暂存、原文件未变。文件系统故障由探针注入,实际复制、权限与发布 owner 未替换。
Windows 本机实测未执行。额外探针按官方要求拒绝普通文件只读句柄的 sync:同输入的基线三种 mode 均失败,当前三种均完成捕获、恢复并保留模式;这是系统调用契约模拟,不是原生 Windows。初版模拟还错误拒绝了 macOS 的目录刷新,已将那次失败保留并修正为只约束普通文件;不能把该探针错误算作产品失败。作者的原生 Windows 结果仅作为作者证据,未冒称独立复现。ACL、硬件断电等平台细节仍需相应平台资格,远程 CI 未查询或等待。
我的整体评价
APPROVE。 这是一处完整且可回滚的文件刷新修复,复用 TypeScript IO owner 和原有见证/发布链,兼容既有只读 POSIX 文件;没有增加手工步骤、持久化状态或另一套恢复权限。长期运行方面保留连续恢复、重放拒绝和失败清理;使用者沿原有显式入口操作即可,不需要第二轮配置或确认。前端、自动普通 Goal 捕获与真实研究长期收益仍是既有文档的独立边界,本次未借用那些资格。没有新共享语义词汇;无需额外框架或迁移层。当前评审是 exact-head 代码结论,不构成合并、升级或全平台安装资格。
English verdict: APPROVE - HEAD 5fe5470; update-handle flushing preserves read-only modes and fails closed through the existing capture/materialize owner. Independent native execution was on macOS; Windows was checked against its documented API contract and an explicit syscall simulation.
Goal And Delivered Outcome
Outcome basis / optional anchor: [Bug]: native Windows: runtime.checkpoint_artifacts capture and materialize always fail because the copied-file durability flush uses a read-only handle #6562 (reproduced defect on native Windows; same flush class as merged fix(backup): flush state backup archives through writable handles on Windows #6258, regressed by merged fix(checkpoint): qualify restored decisions and non-root capture #6415).
Goal/source and gap: on native Windows,
runtime.checkpoint_artifacts- the shipped capture/materialize owner behindloopx research-checkpointand the Codex/research checkpoint hosts - can never capture a checkpoint unit.copyComponentflushes every freshly copied member through a read-only handle (open(destination, "r")+handle.sync()), and WindowsFlushFileBuffersrequires update access, so every capture and materialize aborts withEPERM -4048 fsync. fix(checkpoint): qualify restored decisions and non-root capture #6415 made the flush read-only so an ordinary non-root POSIX controller can capture a read-only participant's0o444copy (r+failsEACCESthere), and the durability flush itself is deliberate (fix(backup): flush state backup archives through writable handles on Windows #6258 fixed the identical flush in thebackup-stateowner). The gap is a flush that must run on every platform while satisfying both access models.Observable before → after, with the validation row that proves it:
node --test --experimental-strip-types tests/control_plane_ts/checkpoint_artifacts.test.tson native Windows: base 5 passed / 8 failed withEPERM -4048 fsyncatcopyComponent; head 14 passed / 0 failed (regression_parityrow). A synthetic product-level capture+materialize throughruntime.checkpoint_artifactswith a read-only participant returnsintegrity_verified: true, preserves the copied and restored0o444mode, andloopx research-checkpoint verifyon the captured unit returnsintegrity_verified: true(real_entrypointrow).Issue/task and intended base: Closes [Bug]: native Windows: runtime.checkpoint_artifacts capture and materialize always fail because the copied-file durability flush uses a read-only handle #6562; base
main. Follow-up to fix(backup): flush state backup archives through writable handles on Windows #6258 (same flush defect class); corrects the regression introduced by fix(checkpoint): qualify restored decisions and non-root capture #6415. Rebased onto32c882fec(after fix(checkpoint): close archive responses before writer cleanup #6569) to satisfy the up-to-date branch ruleset; the changed file content is unchanged.Author Declaration
Implemented against
81ff72b64, the POSIX non-root capture boundary encoded by fix(checkpoint): qualify restored decisions and non-root capture #6415 (its code comment andtests/control_plane/test_checkpoint_restore_capture.py), and the durability contract fix(backup): flush state backup archives through writable handles on Windows #6258 kept for the sibling flush.copyComponent/flushCopiedFile(loopx/control_plane/runtime/checkpoint_artifacts.ts)tests/control_plane_ts/checkpoint_artifacts.test.ts, capture/materialize and SQLite casesflushCopiedFiletests/control_plane/test_checkpoint_restore_capture.pyflushCopiedFiler+ sync denies the Windows flush,r++ sync performs it; a read-only copy syncs after an owner-only grant and the mode is restoredflushCopiedFilenpm run typecheck:control-plane, the focused TS suite, the full TS control-plane suite with a base/head comparison,tests/control_plane/test_checkpoint_restore_capture.py, the two other Python files that call this runtime (test_research_checkpoint_capture.py,test_research_context_adoption.py), and a synthetic end-to-end capture/materialize plus CLIresearch-checkpoint verifyon native Windows. Deliberately left out: a wider sweep for other read-only-handle flushes outside this owner (theopen()audit in fix(backup): flush state backup archives through writable handles on Windows #6258 already covered the sibling; descriptor-created flushes were not reclassified), and a Windows host with symlink privilege, where the guarded assertion would also run. The symlink guard is test-only and uses the established convention from merged test: guard remaining host-dependent control-plane assertions on native Windows #6356 (same comment,process.platform !== "win32"guard).Scope And Continuation
sqlite_backup.tsand this owner was not applied - the SQLite snapshot destination is always created0o600by that owner, so its plainr+open is already the correct narrow contract.Validation
staticpassednpm run typecheck:control-plane: clean.git diff --check: clean.unitpassednode --test --experimental-sqlite --experimental-strip-types tests/control_plane_ts/checkpoint_artifacts.test.ts: 14 passed / 0 failed on native Windows (includes the new read-only participant case). Python twintests/control_plane/test_checkpoint_restore_capture.py: 8 passed.regression_paritypassed81ff72b64vs head: focused suite base 5 passed / 8 failed withEPERM -4048 fsyncatcopyComponent, head 14 passed / 0 failed, and no failing name appears on the head that was absent on the base. Full TS control-plane suite (4776 tests): head 4699 passed / 2 failed / 75 skipped. Both failures are host behavior untouched by this diff:tests/control_plane_ts/zcode_app_server.test.tsprocess_cleanup_unconfirmedreproduces identically on a clean3bc74f931baseline worktree (15 passed / 1 failed there), and atests/control_plane_ts/succession_read_conformance.tsTS-runtime connect timeout under full-suite load passes 1/1 when run in isolation.integrationpassedtests/control_plane/test_research_checkpoint_capture.pyandtests/control_plane/test_research_context_adoption.py(the other Python callers of this runtime) on native Windows: 39 passed / 11 skipped; the 17 failures are host setup limitations that occur before the changed code (16x symlink privilege WinError 1314, 1xos.geteuidabsent on Windows), none in the capture/flush path.real_entrypointpassedruntime.checkpoint_artifactswith a read-only participant returnsintegrity_verified: true,materialized: true, copied and restored mode0o444, restored bytes match;loopx --registry <runtime> research-checkpoint verify --root <unit> --expected-sha256 <digest>returnsintegrity_verified: truewithexecution_authority_granted: false.manualfailedloopx canary premerge --from-git-diff: direct checks, changed-file Python compile and the public/private boundary scan passed; 10 of 19 selected canaries/smokes failed, all outside the changed files, each depending on host prerequisites this machine lacks (a functionalpython3command - it resolves to the non-functional WindowsApps alias, verified by re-runningcatalog-planner-smokesuccessfully via the source-checkout interpreter; symlink privilege; POSIXscripts/loopxshim; packaged install; pnpm workspace; network). Same host-limitation classes recorded for merged siblings #6258/#6464.Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).