CMS-025A — Safe atomic PGN export writes

[levallem]

Context

Audited against current main:

3dd593c9ba8206b1d5a8fa8fc592229cdb1ed7f7

This is the merge of CMS-024N / PR #37.

The Astra audit identified AUD-001 — Sobrescritura PGN no atómica:

• severity: Medium;
• category: Data Integrity;
• status: Confirmed;
• priority: P1.

The current PGN writers build/validate the complete PGN content first, but then publish directly to the requested destination with std::fs::write(...).

Current public APIs:

pub fn write_pgn(
    puzzles: &[config::Puzzle],
    path: &Path,
) -> Result<(), String>

pub fn write_project_pgn(
    project_name: &str,
    chapters: &[ProjectPgnChapter],
    path: &Path,
) -> Result<(), String>

pub fn to_pgn(
    puzzles: &[config::Puzzle],
    _lang: &lang::Language,
    path: String,
) -> Result<(), String>

Runtime callers already converge on those writers:

src/main.rs
  ExportPGN
  → export::to_pgn()
  → write_pgn()

src/project_tab.rs
  export_selected_puzzles_pgn()
  → write_pgn()

  export_project_pgn()
  → write_project_pgn()

  export_selected_chapters_pgn()
  → write_project_pgn()

Therefore this task should harden the writers internally without changing the caller APIs.

Objective

Prevent a failed PGN export from truncating or partially overwriting an existing destination file.

The new publication flow must be conceptually:

build complete PGN content
→ create unique sibling temporary file
→ write complete bytes
→ sync temporary file
→ close temporary file
→ rename temporary file over destination

The existing destination must not be deliberately modified or removed before the final publication step.

Required publication semantics

Implement one small internal helper in src/export.rs used by both PGN writers.

The helper should:

1. derive a temporary path in the same directory as the destination;
2. create the temporary file with OpenOptions::create_new(true);
3. avoid clobbering an already-existing temporary file;
4. write all bytes with write_all;
5. call sync_all();
6. close/drop the file handle before publication;
7. publish with std::fs::rename(temp, destination);
8. remove the temporary file best-effort when an error happens before successful publication;
9. return an error that includes useful destination/path context.

A reasonable temporary naming scheme is conceptually:

<destination filename>.cms-<pid>-<counter>.tmp

Exact naming may follow existing project style, but it must allow retry/collision avoidance and must not overwrite a stale temporary file.

Do not use a fixed sibling .tmp path if that can be silently truncated or can block subsequent exports after an interrupted previous run.

Existing destination behavior

Destination does not exist

On success:

• publish the complete PGN at the requested destination;
• no temporary file remains.

On failure before publication:

• destination remains absent;
• temporary file is cleaned best-effort.

Destination already exists

Until the final publication step:

• existing destination bytes must remain unchanged.

On successful publication:

• destination contains only the new complete PGN.

On a detected failure before successful publication:

• return Err;
• do not fabricate success;
• do not intentionally truncate or remove the existing destination;
• clean temporary output best-effort.

Atomicity / durability contract

This task must not claim stronger crash-durability guarantees than the implementation proves.

The intended guarantee is:

CMS builds and writes the new PGN into a sibling temporary file and only attempts to replace the destination after that temporary file has been fully written and synchronized.

Do not promise:

the destination is guaranteed to survive every possible power loss, kernel crash, filesystem failure, or hardware failure.

Portable directory-fsync / full power-loss durability is outside this task.

The goal is safe publication against application-visible build/write/sync/publication failures, with the previous destination not deliberately truncated during preparation.

Windows portability

The implementation must remain portable and use the Rust standard library only.

Requirements:

• sibling temporary file, so source and destination are on the same filesystem;
• close the temporary file before rename;
• never delete the destination first just to make the rename work;
• no Windows-specific FFI;
• no ReplaceFileW;
• no new crate solely for this task.

If implementation reveals a real cross-platform blocker that cannot be solved within these constraints, stop and report it instead of adding platform-specific complexity silently.

Reuse existing project patterns

Chess Material Studio already has related safe-write patterns:

• src/config.rs writes to a sibling temporary file, syncs, closes, then renames;
• src/download_db.rs uses a backup/publish/restore flow for a different use case.

For CMS-025A, prefer the simpler write sibling temp → sync → close → rename shape.

Do not introduce a destination → backup → new destination sequence unless a demonstrated platform requirement forces it.

Scope

Expected production file:

src/export.rs

Tests should remain in the existing src/export.rs test module unless there is a strong reason otherwise.

No caller change is expected.

Do not modify

Do not modify unless a real blocker is found:

src/main.rs
src/project_tab.rs
src/project.rs
src/pgn_import.rs
src/pgn_tab.rs
src/pgn_review.rs
src/settings.rs
src/config.rs
src/download_db.rs
src/schema.rs
project_migrations/
translations/
README.md
Cargo.toml
Cargo.lock

If a scoped-out file genuinely becomes necessary, stop and report why before broadening the task.

No dependency changes

Do not add:

• tempfile;
• Windows API crates;
• new filesystem crates;
• any other dependency for this feature.

Use the standard library and existing dependencies only.

Public API compatibility

Preserve the current signatures and behavior contract of:

write_pgn(...)
write_project_pgn(...)
to_pgn(...)

Callers in main.rs and project_tab.rs should not require production changes.

Existing user-facing success/error propagation must continue to work.

Tests

Add focused regression tests around the shared safe-publication helper and the two writers.

At minimum cover:

Existing destination is replaced only after success

• seed destination with known sentinel bytes;
• perform a valid export;
• verify final destination contains the new complete PGN;
• verify old sentinel content is gone;
• verify no temporary file remains.

Build failure preserves existing destination

• seed destination with known bytes;
• use an invalid puzzle that makes PGN construction fail;
• verify write_pgn(...) returns Err;
• verify destination bytes are unchanged.

Also preserve the existing equivalent project-PGN invalid-game behavior.

Publication failure preserves existing destination

Provide a small test seam around the final rename/publication step if needed.

Inject a deterministic publication failure and verify:

• writer/helper returns Err;
• previous destination bytes remain unchanged;
• no success is fabricated;
• temporary output is cleaned best-effort.

Keep the injection local to src/export.rs; do not create generic infrastructure.

Temporary creation/write failure

Create a deterministic failure before publication.

Verify:

• existing destination remains unchanged;
• error propagates;
• no destination truncation occurs.

New destination

When no destination exists:

• valid export succeeds;
• destination contains complete PGN;
• no temporary file remains.

Stale temporary collision

Create a file using the first candidate temporary name or otherwise force a collision.

Verify:

• the stale file is not truncated;
• export can choose another candidate and succeed, or returns a controlled error according to the helper contract;
• destination safety is preserved.

Both PGN writers use the safe path

Keep at least one integration-level regression for:

• write_pgn;
• write_project_pgn.

Do not duplicate every helper-level failure test for both APIs.

Existing tests to preserve

Current tests already cover:

• received-order preservation;
• build/write error propagation;
• editorial tags;
• PGN tag escaping;
• invalid game rejection;
• normal export wrapper behavior.

These must remain green.

Acceptance criteria

• write_pgn no longer calls std::fs::write directly on the destination.
• write_project_pgn no longer calls std::fs::write directly on the destination.
• Both writers use one shared internal safe-publication helper.
• PGN content is still built completely before filesystem publication starts.
• Temporary file is a sibling of the destination.
• Temporary creation cannot silently clobber an existing temporary file.
• Complete bytes are written with write_all.
• Temporary file is sync_all()'d before publication.
• Temporary handle is closed before rename.
• Destination is never deleted first as part of normal replacement.
• Existing destination remains byte-for-byte unchanged on deterministic pre-publication failures.
• Deterministic publication failure returns Err without fabricating success.
• Successful replacement yields the complete new PGN.
• Successful creation works when the destination did not previously exist.
• Temporary files are cleaned best-effort on ordinary detected failures.
• No new dependency is added.
• Public PGN writer signatures remain unchanged.
• No production caller change is required.
• PDF export behavior is unchanged.
• Project SQLite/schema/migrations are unchanged.
• Existing and new tests are green.
• Cargo.toml and Cargo.lock remain unchanged.
• Production diff is limited to src/export.rs, unless a blocker is reported first.

Verification

Run:

git status
cargo fmt
cargo fmt --check
cargo clippy --all-targets --all-features -- -D warnings
cargo test
cargo check
cargo build
git diff --check
git diff --name-only
git diff -- src/export.rs
git status

Also run the focused CMS-025A tests explicitly if useful during development.

Because the current CI runs tests on Ubuntu but Windows jobs primarily build/package, perform a local Windows cargo test before considering the task complete.

Out of scope

Do not implement:

• safe PDF replacement;
• generic filesystem abstraction;
• generic transaction framework for files;
• backup/restore UI;
• PGN parser changes;
• PGN terminal result reconciliation;
• resource/CWD fixes;
• stale search fixes;
• UCI evaluation changes;
• schema or migration changes;
• project persistence changes;
• caller/UI refactors;
• unrelated cleanup;
• large refactors of export.rs.

Completion report

Before finishing, report:

1. exact files changed;
2. exact helper/publication design used;
3. tests added;
4. commands executed and results;
5. whether Cargo.toml / Cargo.lock changed;
6. whether any scoped-out file had to be touched;
7. git diff --check result;
8. final git status.

Do not commit, push, open a PR, merge, or modify GitHub metadata beyond this implementation task without explicit authorization.