fix: make file replacements atomic - #568
Conversation
Stage single-file writes beside their destinations and preserve Unix creation permissions. Commit ETag-backed downloads before updating validators so failed transfers keep the previous cache usable.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #568 +/- ##
==========================================
+ Coverage 72.07% 72.71% +0.64%
==========================================
Files 72 72
Lines 6679 6854 +175
Branches 6679 6854 +175
==========================================
+ Hits 4814 4984 +170
Misses 1523 1523
- Partials 342 347 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR centralizes atomic single-file replacement into a new workspace crate (maa-atomic-fs) and adopts it across the CLI, installer download cache, and release metadata generation to avoid corrupting existing files on interrupted downloads or failed writes.
Changes:
- Added
maa-atomic-fsproviding same-directory staging, fsync, and atomic replacement with Unix permission handling. - Updated
maa-installerETag download path to commit bodies before validators and to remove stale ETag sidecars when omitted. - Migrated
maa-cliconfig/import/migration/Copilot cache writes andxtaskrelease metadata writes to the shared helper (removing the old CLI-local atomic fs module).
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| xtask/src/release/package.rs | Writes release JSON and shell metadata via maa-atomic-fs and adds regression coverage for replacements. |
| xtask/Cargo.toml | Adds maa-atomic-fs dependency to xtask. |
| crates/maa-installer/src/download/etag.rs | Uses atomic writes for cached downloads/ETags; adjusts ETag sidecar update/removal and timestamp refresh logic; adds regression tests. |
| crates/maa-installer/Cargo.toml | Adds maa-atomic-fs dependency to installer crate. |
| crates/maa-cli/src/run/preset/copilot.rs | Switches Copilot cache writes to maa-atomic-fs. |
| crates/maa-cli/src/main.rs | Removes the old atomic_fs module wiring. |
| crates/maa-cli/src/config/mod.rs | Uses maa-atomic-fs for config file writes. |
| crates/maa-cli/src/config/import.rs | Uses maa-atomic-fs for imported config writes/copies. |
| crates/maa-cli/src/config/asst.rs | Uses maa-atomic-fs for legacy tasks migration copy. |
| crates/maa-cli/src/atomic_fs.rs | Removes the CLI-local atomic fs implementation (superseded by shared crate). |
| crates/maa-cli/Cargo.toml | Adds maa-atomic-fs dependency to CLI crate. |
| crates/maa-atomic-fs/src/lib.rs | Introduces the shared atomic file replacement implementation and unit tests. |
| crates/maa-atomic-fs/Cargo.toml | Defines the new maa-atomic-fs crate. |
| Cargo.toml | Registers maa-atomic-fs as a workspace dependency. |
| Cargo.lock | Locks the new crate into the workspace dependency graph. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
crates/maa-installer/src/download/etag.rs:87
- The trace log in the 304/Not Modified branch says it "Touched" the destination file, but the code actually updates the mtime on the ETag sidecar file. This can make debugging cache freshness behavior confusing when reading logs.
StatusCode::NOT_MODIFIED => {
log::trace!("File {} is up to date", dest.display());
if set_modified(&etag_file, time::SystemTime::now()).is_ok() {
log::trace!("Touched {}", dest.display());
}
Summary
Prevent interrupted downloads and failed writes from replacing valid files with partial content, and centralize the single-file replacement contract in a reusable workspace crate.
Changes
maa-atomic-fsfor same-directory staging, file synchronization, and atomic replacement of existing destinationsThe helper intentionally does not provide multi-file transactions, writer locking, metadata preservation beyond documented Unix permission bits, or parent-directory synchronization. Windows replacement uses the overwrite-capable persistence path from
tempfile, covered by the Windows CI jobs.Validation
cargo +nightly fmt --checkcargo test -p maa-atomic-fs(9 passed)cargo test -p maa-installer --features ureq/json(65 passed)cargo test -p xtask(1 passed)cargo x test --no-core-tests由 Sourcery 生成的摘要
在新的
maa-atomic-fscrate 中集中实现原子化的单文件替换,并将其应用到下载、配置写入、缓存文件和发布元数据,以在失败时避免破坏已有文件,同时保留预期的权限和 ETag 语义。新增功能:
maa-atomic-fs,提供可复用的原子化单文件写入、拷贝和暂存工具缺陷修复:
改进:
maa-cli配置、迁移、Copilot 缓存写入、安装程序下载以及xtask发布元数据生成中,统一采用共享的原子化文件替换辅助工具测试:
Original summary in English
Summary by Sourcery
集中管理单文件的原子替换,并迁移写入文件的流程,以保护现有内容不被部分或失败的更新覆盖。
新特性:
错误修复:
改进优化:
构建:
maa-atomic-fscrate 以及相关 workspace 依赖。测试:
Original summary in English
Summary by Sourcery
Centralize single-file atomic replacement and migrate file-writing paths to protect existing content from partial or failed updates.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: