Repository navigation
fix(backup): flush state backup archives through writable handles on Windows - #6258
Conversation
…Windows
On native Windows `loopx backup-state --execute` cannot complete. The SQLite
snapshot owner flushes the freshly written copy through a read-only handle
(`open(destination, "r")` + `sync()`), and Windows flushes require write
access, so every state tree that contains a SQLite file aborts with
`EPERM -4048 fsync`. Without SQLite in scope the archive owner then reopens
the staged archive and manifest read-only (`path.open("rb")`) before
`os.fsync`, which Windows rejects with `EBADF Errno 9`. The user sees only
"Effect runtime filesystem operation failed" or "[Errno 9] Bad file
descriptor", and the recommended pre-reconnect state backup never happens.
Both flushes are deliberate durability steps, so keep them and open the
handles for update (`"r+"` / `"rb+"`) on every platform instead of dropping
the flush on Windows. The SQLite snapshot regression case also asserted the
POSIX owner permission bit, which Windows stat reports as 0o666 for every
writable file, so that line is skipped where the host has no such mode.
Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
`test_duplicate_targets_reuse_snapshot_and_preserve_ordinary_files` asserts the staged archive and manifest report mode 0o600, which Windows stat cannot express: it reports 0o666 for every writable file. The case only reaches that line on Windows after the durability flush fix, so a host with symlink privilege would have failed there instead of at the pre-existing symlink setup. Guard the assertion the same way as its TypeScript counterpart. Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
|
Independent review disposition (reviewing agent, on head
AI disclosure: this review and this comment were produced with AI assistance at the operator's request, under their direction and review. |
There was a problem hiding this comment.
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=xhigh
APPROVE. 精确 head 327c0a756510b9140f695a19eb310327b7ded436。
动机
在 Windows 上执行本地状态备份、需要取得可验证备份文件的维护者。
用户执行 backup-state 时，旧版用只读文件句柄请求强制落盘；Windows 会返回 EPERM 或 EBADF，SQLite 快照、压缩归档或清单不能完成。修复改用可写句柄后，仍先校验与落盘再发布备份，用户能取得可回读的归档和摘要清单。
真实 CLI 在含 SQLite 和仅普通文件的布局中生成归档与匹配清单，恢复数据库内容及文件摘要可读。Windows 可写句柄约束模型区分 base 的失败与 head/wheel 的成功；dry-run、损坏 SQLite 和暂存落盘失败均不发布备份。
本批修复既有本地备份的跨平台落盘方式；不自动恢复或激活备份、不扩大读取范围或权限，也不宣称本评审运行过原生 Windows、ACL 安全资格或 PostgreSQL 存储迁移。
改动思路
保持两个既有 IO owner 的校验与 fsync 顺序，只把句柄改为 r+/rb+，即可满足 Windows 的可写要求；删掉落盘或吞掉异常会削弱持久性。SQLite 快照仍由共享 TS owner 提供，Python 仅负责归档字节，不新增平行存储决策源。
当前 PR 一起修复 SQLite 快照、归档、清单三处落盘及两份测试的 Windows 权限断言；不改备份格式、发现范围、恢复权限或存储选型。
具体改动
spec_ref：https://github.com/loopx-project/loopx/issues/6252；spec_revision：aedb376db32a2d8a2e93686ee3800715463654e0。Issue6252 创建/更新于2026-10-11T17:01:52Z；同时对照该 merge base 的 configuration-backup 合同。本轮对照的 main commit 为 c51ec1a，相关合同未变化；不是把它误当作此分支的祖先。
writable-durable-snapshot：implemented；The existing online SQLite snapshot passes integrity check and flushes through a writable handle before use.verified-archive-manifest：implemented；Backups with and without SQLite flush archive/manifest and expose readback hashes for the copied bytes.failure-publication-boundary：implemented；Dry-run has no archive effects; corrupt snapshots or staged durability failure do not publish partial output; existing backup preservation remains.platform-permission-contract：implemented；POSIX private output mode remains 0600; Windows does not assert POSIX mode bits as an ACL guarantee.
关键代码讲解
snapshotSqliteDatabase将已预留快照文件的open("r")改为open("r+")；在线 backup、完整性校验、sync、finally close 与失败删除都保留。共享的 authority-format-upgrade 调用也直接复用它。execute_state_backup_plan将已设私有权限的暂存归档与清单由path.open("rb")改为path.open("rb+")；os.fsync成功后才到原来的os.replace发布，不用重试或静默降级隐藏失败。- 两份测试仅在非 Windows 时断言 0600；文件摘要、成员内容、数据库完整性、覆盖保护及回读断言仍执行。Windows stat mode 不是 POSIX 权限承诺，这不是扩大读取或发布范围。
- 使用真实 Node SQLite owner 与真实 CLI 分别验收两层落盘，再通过正常前端资产构建、wheel 打包与隔离安装验证组合入口；未替换数据库或 fsync 为成功 mock。
对主干的风险
19 项真实备份/成员完整性/来源保留 pytest、14 项实际 Node SQLite 快照与 authority-format-upgrade 测试、control-plane typecheck、公共 state-backup smoke、Ruff、diff、语义与 IO manifest 检查通过。冻结 CLI oracle：base 5 通过、2 个 Windows Python 可写句柄用例失败；head 与 wheel 各 7/7 通过。实际 Node 可写约束探针 base 2 通过/1 失败，head 3/3 通过。
本机不是 Windows。fcntl 实际句柄访问模式与真实 Node FileHandle.sync 的约束模型验证可写条件，但不等于原生 Windows、junction、ACL、进程池或崩溃耐久资格。作者提供的 Windows 整套测试红项与 focal 结果没有由本评审独立运行；本次未查询 CI，也不把评审通过当作合并就绪。未触及 PostgreSQL provider。
正常构建前端 chat assets 后打包，安装到隔离环境，在 checkout 外用 python -I 验证模块来源和实际调用；未绕过包装要求。语义 advisory 未检测到支持的新词汇载体；完整语义 smoke 通过但45个 unresolved producer sites仍保留，不能宣称全树语义已证明。IO manifest291站点检查通过。初始探针错误把 config 同步也计成两个暂存 fsync，且把 Node preload 注入 IPC 启动造成两版本探针故障；日志保留。纠正探针后，真实 CLI 的最终暂存句柄、故障发布保护及独立 Node owner 约束仍完整验收，不宣称旧 IPC 探针被产品修好。 作者声明与本评审独立证据分开，CI未查询、轮询或等待。
我的整体评价
未发现这一有界修复的阻塞问题。保持两个既有 IO owner 的校验与 fsync 顺序，只把句柄改为 r+/rb+，即可满足 Windows 的可写要求；删掉落盘或吞掉异常会削弱持久性。SQLite 快照仍由共享 TS owner 提供，Python 仅负责归档字节，不新增平行存储决策源。 当前 PR 一起修复 SQLite 快照、归档、清单三处落盘及两份测试的 Windows 权限断言；不改备份格式、发现范围、恢复权限或存储选型。 未来维护整理已检查：两个当前消费者继续复用同一 SQLite owner，无需为两个打开模式增加通用 helper。 用户的现有操作路径没有增加输入、确认或授权步骤，失败与下一次工作仍能诚实回读。阶段恢复经验仅作提示，不继承历史 approval，不宣布母路线图完成、生产采用或合并就绪。
English verdict: APPROVE - exact head 327c0a7. Writable flush handles repair the reproduced durability precondition without bypassing sync, integrity or staged publication; native Windows not independently run.
|
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=xhigh English verdict: APPROVE - exact head 327c0a7. Writable flush handles repair the reproduced durability precondition without bypassing sync, integrity or staged publication; native Windows not independently run. MotivationA Windows maintainer runs backup-state and expects a readable archive plus hash manifest. Readonly handles previously caused EPERM in the Node SQLite snapshot sync and EBADF in Python archive/manifest fsync. The candidate uses writable handles while retaining verification and forced durability before publication. Actual CLI stages a byte-witness archive with/without real SQLite, reads SHA-256 and file-member byte counts, extracts and reopens a database with fixed expected rows. Source and isolated wheel pass; actual shared SQLite format-upgrade caller tests pass. The scope is this existing bounded repair; it does not certify parent roadmap completion, production adoption or new authority. Approach and ownershipThe existing TS online SQLite owner remains shared by state backup and authority-format upgrade. Python retains the specialized private archive publisher. Change r to r+ and rb to rb+, rather than dropping fsync, swallowing errors, introducing platform fallback or duplicating storage policy. All three flush points and both platform mode assertions are delivered together. The current PR boundary is the SQLite snapshot, archive and manifest writable flushes plus correct platform assertions; no format, discovery, restore or provider-selection changes. Specification and code walkthroughspec_ref:
The SQLite path still reserves a private target, performs online backup, removes journal leftovers, checks integrity, syncs and closes; failure removes its created output. The archive publisher still copies actual bytes, records member witnesses, chmods private staging, fsyncs both files and then performs the existing replacements. Windows tests stop asserting POSIX mode bits; POSIX0600 and all byte/database/readback assertions remain. The shared format-upgrade consumer is unchanged and tested with actual SQLite. Positive walkthrough: Actual CLI stages a byte-witness archive with/without real SQLite, reads SHA-256 and file-member byte counts, extracts and reopens a database with fixed expected rows. Source and isolated wheel pass; actual shared SQLite format-upgrade caller tests pass. Negative walkthrough: Dry-run creates no output directory; corrupt SQLite refuses publication; deliberately fail the staged archive fsync after config synchronization and assert neither archive nor manifest publishes. Existing pytest also preserves a prior backup on corrupt source failure. Validation and risks to main19 actual backup/member-integrity/retention pytest cases,14 real Node SQLite/format-upgrade tests,control-plane typecheck,the public backup smoke,Ruff,diff checks,semantic smoke and291-site IO manifest passed. Frozen actual CLI cases cover ordinary bytes/SQLite, hashes and restored rows,dry-run,corrupt DB and staged-fsync publication failure. Immutable base passes5 controls and fails2 simulated Python writable-handle cases; exact head and separately installed wheel each pass7/7. A real FileHandle access-mode guard with actual sync call-through independently yields base2 pass/1 readonlyEPERM and head3/3. Normal chat asset build and ordinary wheel packaging preceded isolated installation. Outside-checkout The reviewer did not run native Windows. The handle constraint model is narrower than Windows ACL,junction,crash durability or process-pool qualification. Author-provided native-Windows suite failures/claims are not independent reviewer evidence; CI was not consulted and merge readiness is separate. No PostgreSQL provider changes. Initial probe failures are preserved: its expected descriptor count overlooked an earlier config sync, and Node preload IPC startup did not produce a verifiable response on either base/head. The corrected immutable probe retains final staged handle and failure-publication assertions; direct Node-owner tests plus ordinary real CLI/wheel qualify the changed composition without claiming that old IPC failure was fixed. Overall assessmentAPPROVE for the exact head, with no identified blocker in this bounded slice. Sustained recovery/next-operation and truthful failure/readback are preserved without extra user input or approval steps. The future-facing pass found the existing shared SQLite owner and private staged publisher sufficient; no speculative helper or protocol is needed. Current repository experience was advisory and independently checked, not inherited proof. This approval is not a native-Windows qualification or permission to merge. |
3 participants
Goal And Delivered Outcome
Outcome basis / optional anchor: [Bug]: native Windows: loopx backup-state --execute always fails because both durability fsync calls use read-only file handles #6252 (reproduced defect on native Windows).
Goal/source and gap: on native Windows,
loopx backup-state --execute- the state-protection command the bootstrap command pack tells the operator to run before a destructive reconnect - cannot complete. The backup path performs two durability flushes and both open the target read-only.snapshotSqliteDatabaseflushes the freshly written copy throughopen(destination, "r")+handle.sync(); Windows rejects that withEPERM -4048 fsync, so any state tree containing an SQLite file aborts.execute_state_backup_planthen reopens the staged archive and manifest withpath.open("rb")beforeos.fsync; Windows rejects that withEBADF Errno 9, so even a backup without SQLite fails. The operator sees onlyEffect runtime filesystem operation failedor[Errno 9] Bad file descriptor, and the recommended backup never happens.Observable before → after, with the validation row that proves it: the same synthetic-state CLI invocation exits 1 with
"error": "Effect runtime filesystem operation failed"(SQLite in scope) and 1 with"error": "[Errno 9] Bad file descriptor"(no SQLite) before, and exits 0 with"ok": true, a written archive/manifest and a recordedsqlite_snapshotsentry after (real_entrypointrow). The repository's own suites move from failing to passing on Windows: the SQLite snapshot case intests/control_plane_ts/sqlite_backup.test.tsand 28 backup-related Python tests (regression_parityrow).Issue/task and intended base: Closes [Bug]: native Windows: loopx backup-state --execute always fails because both durability fsync calls use read-only file handles #6252; base
main.Author Declaration
Implemented against
aedb376db, plus the durability contract the two flush sites already encode.snapshotSqliteDatabase(loopx/control_plane/coordination/sqlite_backup.ts)tests/control_plane_ts/sqlite_backup.test.ts, first caseexecute_state_backup_plan(loopx/state_backup.py)tests/test_state_backup.pyjunction and real-CLI casesopen(destination, "r+"),path.open("rb+")r/rbdeny the flush on Windows,r+/rb+perform itnpm run typecheck:control-plane, the focused pytest files, the full TypeScript control-plane suite with a base/head failure-set comparison,loopx checkpublic/private boundary overloopx/ tests/ examples/ docs/, andloopx canary premerge --from-git-diff. Deliberately left out: the unrelated host limitations named under Validation, and a wider sweep for the same read-only flush pattern outside the backup path (the explicitopen()audit found no other file-level read-only flush; descriptor-created flushes, mostly directory flushes, were not reclassified), and an independent review probe for the"r+"open on a non-writable destination (dismissed: the precedingdriver.backupwrite already requires a writable destination, so that state is unreachable here).Scope And Continuation
backup-statejourney now use an update handle; the bytes written and the durability guarantee are unchanged.Validation
staticpassedruff check loopx/state_backup.py tests/test_state_backup.py: All checks passed.npm run typecheck:control-plane: clean.git diff --check: clean.unitpassednode --test --experimental-sqlite --experimental-strip-types tests/control_plane_ts/sqlite_backup.test.ts- the SQLite snapshot and invalid-request cases pass on native Windows. Python:tests/test_state_backup.py5 passed, 1 failed for the pre-existing host limit below.regression_paritypassedaedb376dbvs head on the same native-Windows host. Full TS control-plane suite (4453 tests): base 69 failed / 4324 passed, head 66 failed / 4327 passed, and no failing name appears on the head that was absent on the base. Backup-related Python files (same set both sides): base 63 failed / 25 passed / 2 skipped, head 35 failed / 53 passed / 2 skipped, again with no new failing name. Reverting only the two product lines restores the originalEPERM/EBADFfailures.real_entrypointpassedpython -m loopx.cli --format json --runtime-root <runtime> backup-state --project <project> --output-dir <backups> --no-automations --no-skills --current-project-only --executeexits 1 with"error": "Effect runtime filesystem operation failed"(SQLite in scope) and 1 with"error": "[Errno 9] Bad file descriptor"(no SQLite) on the base, and exits 0 with"ok": true, a written archive and manifest, and one recordedsqlite_snapshotsentry on the head.manualfailedpython -m mypy: one error,loopx/control_plane/effect_runtime.py:432: Module has no attribute "WNOHANG", reproduced identically with the change stashed. Pre-existing Windows-host limitation, not a diff effect.manualfailedloopx canary premerge --from-git-diff: diff hygiene, changed-file compile and public/private boundary checks passed; 12 of 19 selected checks passed and 7 failed. Each failing smoke was re-run with the change stashed and failed identically (Windows symlink privilege, missingbash, smoke-local Windows path/newline assumptions), i.e. host environment, not a diff effect.windows-powershelllane does not run either affected suite, which is how this shipped. The flush open is only reached for a destination the process can write, becausedriver.backupmust open that database for update first: a probe that makes the destination read-only fails withattempt to write a readonly databasebefore the flush, so the writable-handle change adds no new failure mode for a narrowing POSIX umask. Untested here: a Windows host with symlink privilege, where the second TS case would also run; on this host it stops atsymlinkbefore reaching backup code, identically with the change stashed.Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Literal/closed-set vocabulary or serialization contract changed, so the diff-scoped semantic inventory advisory is not applicable.Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).