Skip to content

fix(checkpoint): flush captured members through update handles on Windows - #6566

Open
JasonBuildAI wants to merge 1 commit into
loopx-project:mainfrom
JasonBuildAI:codex/agent-z4k9
Open

JasonBuildAI wants to merge 1 commit into
loopx-project:mainfrom
JasonBuildAI:codex/agent-z4k9

Conversation

@JasonBuildAI

@JasonBuildAI JasonBuildAI commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

Author Declaration

  • Written by: an AI coding agent (DeepSeek V4.1 Flash Free, OpenCode), at the human operator's direction and under their review. AI disclosure: this change was drafted with AI assistance at the operator's request, under their direction and review.

Implemented against

Criterion (spec clause) Disposition Symbol / path Test or command
A capture on native Windows publishes the unit instead of aborting implemented copyComponent/flushCopiedFile (loopx/control_plane/runtime/checkpoint_artifacts.ts) tests/control_plane_ts/checkpoint_artifacts.test.ts, capture/materialize and SQLite cases
A read-only participant's copied mode (0o444) still survives capture and materialize implemented flushCopiedFile new read-only participant case; tests/control_plane/test_checkpoint_restore_capture.py
The durability flush is still performed on every platform rather than skipped on Windows implemented flushCopiedFile host probe: r + sync denies the Windows flush, r+ + sync performs it; a read-only copy syncs after an owner-only grant and the mode is restored
No POSIX behavior change for writable copies implemented flushCopiedFile base/head full-suite comparison below; Linux CI exercises the POSIX branch
  • Self-check before submission: ran npm 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 CLI research-checkpoint verify on native Windows. Deliberately left out: a wider sweep for other read-only-handle flushes outside this owner (the open() 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

  • Completed scope and remaining work: complete within this scope. The single flush call site now keeps its durability guarantee on Windows and POSIX, and the reviewed mode is restored before the copy is witnessed or published.
  • Slice boundary / successor: complete within this scope; no successor needed. Bounded future-facing refactoring considered: a shared "flush a possibly read-only file" helper for sqlite_backup.ts and this owner was not applied - the SQLite snapshot destination is always created 0o600 by that owner, so its plain r+ open is already the correct narrow contract.

Validation

  • Tested revision: 5fe5470
  • Run state: finished
  • Input classes: synthetic
Check kind Result Public-safe evidence / limitation
static passed npm run typecheck:control-plane: clean. git diff --check: clean.
unit passed node --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 twin tests/control_plane/test_checkpoint_restore_capture.py: 8 passed.
regression_parity passed Same native-Windows host, base 81ff72b64 vs head: focused suite base 5 passed / 8 failed with EPERM -4048 fsync at copyComponent, 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.ts process_cleanup_unconfirmed reproduces identically on a clean 3bc74f931 baseline worktree (15 passed / 1 failed there), and a tests/control_plane_ts/succession_read_conformance.ts TS-runtime connect timeout under full-suite load passes 1/1 when run in isolation.
integration passed tests/control_plane/test_research_checkpoint_capture.py and tests/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, 1x os.geteuid absent on Windows), none in the capture/flush path.
real_entrypoint passed Native Windows, synthetic runtime root: capture + materialize through runtime.checkpoint_artifacts with a read-only participant returns integrity_verified: true, materialized: true, copied and restored mode 0o444, restored bytes match; loopx --registry <runtime> research-checkpoint verify --root <unit> --expected-sha256 <digest> returns integrity_verified: true with execution_authority_granted: false.
manual failed loopx 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 functional python3 command - it resolves to the non-functional WindowsApps alias, verified by re-running catalog-planner-smoke successfully via the source-checkout interpreter; symlink privilege; POSIX scripts/loopx shim; packaged install; pnpm workspace; network). Same host-limitation classes recorded for merged siblings #6258/#6464.
  • Coverage and gaps: both changed product lines are exercised on native Windows by the focused suite (writable copies take the update path; the new case takes the read-only grant path) and on Linux CI, where the POSIX branch keeps its previous effect. The Windows-only failure cannot be reproduced by Linux CI, which is how it shipped. Untested here: a Windows host with symlink privilege (the guarded assertion would run there) and a POSIX host running as an unprivileged user (covered by the existing Python twin test on CI, which chmods the source to 0o444).

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • States and viewports shown: N/A
  • Source data: none

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring (no functional changes)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)

Technical Direction

Shared-authority RFC fixture impact

  • Production-scale fixture schema: N/A
  • Semantic dimensions changed, or reviewed no-impact rationale: N/A - no authority/state vocabulary change; the diff is a filesystem durability detail.
  • Provider conformance arms run: N/A
  • Read-only legacy/file/PostgreSQL three-arm rehearsal (required for promotion, runtime-routing, or compatibility-projection changes): N/A

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths (including .loopx/, .codex/goals/, and live ACTIVE_GOAL_STATE.md).
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.
  • Every commit includes a DCO Signed-off-by trailer (git commit -s).

@JasonBuildAI JasonBuildAI left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

  1. Root cause confirmed. fs.open(copiedPath, "r") + fsync fails EPERM -4048 on Windows for every copied file because FlushFileBuffers requires a write-capable handle. Focused suite on base 3bc74f931: 5 passed / 8 failed (EPERM -4048 at copyComponent); on this head: 14 passed / 0 failed.
  2. Fix semantics correct on both platforms. Writable copies flush through an r+ handle. On EACCES/EPERM the copy's mode is read via lstat, granted mode | 0o600, flushed via r+, then restored with chmod(path, mode) in finally — 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.
  3. Adversarial probe (independently re-run). Real runtime.checkpoint_artifacts capture + materialize + verify with a read-only participant (0o444) and a standalone 0o400 file:
    • capture integrity: true, source stability: true;
    • artifact_witness modes equal source_witness modes (444/666 preserved);
    • materialize integrity_verified: true, restored modes and bytes match;
    • verify integrity_verified: true;
    • missing-source probe rejects ENOENT (fail-fast intact).
  4. 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.
  5. Sibling path unchanged. The SQLite snapshot destination in sqlite_backup.ts is always created 0o600, so its plain r+ 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-powershell CI 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.ts process_cleanup_unconfirmed also fails on clean base 3bc74f931; succession_read_conformance.ts passes 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.

JasonBuildAI added a commit to JasonBuildAI/loopx that referenced this pull request Oct 15, 2026
…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>
JasonBuildAI added a commit to JasonBuildAI/loopx that referenced this pull request Oct 15, 2026
Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
@JasonBuildAI

Copy link
Copy Markdown
Contributor Author

Rebase note — head updated to 5fe54703c80538e87def0be20f812ea18986434b.

The main branch ruleset requires an up-to-date branch (strict_required_status_checks_policy), so I rebased onto main 32c882fec (after #6569). The changed content is identical (+47/−8, same two files; #6569 touched only benchmark/edgebench checkpoint files). Re-verified on the new base on native Windows: focused suite 14/14, npm run typecheck:control-plane clean, and the adversarial capture/materialize/verify probe produced identical results (integrity_verified: true, 0o444 modes preserved, missing-source probe still rejects ENOENT).

The COMMENTED review above remains attached to the previous head e6d3b4728; its findings apply unchanged. CI is re-running on the new head.

@loopx-agent loopx-agent 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.

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;恢复历史不赋予当前执行资格。

关键代码讲解

  1. syncCopiedFile,第117行:r+ 打开已复制文件,sync 后在 finally 关闭句柄;文件刷新失败不会被当成成功。
  2. flushCopiedFile,第128行:仅 EACCES/EPERM 进入临时改权路径,保存原 mode,并在重试后恢复;操作对象是新建0700目录内的副本。
  3. 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.

This branch has not been deployed

No deployments
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]: native Windows: runtime.checkpoint_artifacts capture and materialize always fail because the copied-file durability flush uses a read-only handle

2 participants