fix: atomic, encoding-preserving .al writes with batch rollback for apply_fix and apply_fix_all

[Arthurvdv]

Summary

apply_fix and apply_fix_all are the only tools that write .al files. On main the write was a truncating in-place File.WriteAllTextAsync with the default encoding, so a UTF-8-with-BOM file lost its BOM (whole-file git diff, git blame destroyed), a UTF-16 file was silently converted to UTF-8, a crash mid-write could leave a truncated .al file, and a failed write in apply_fix_all left the earlier files of the batch modified.

This PR makes every write atomic, encoding-preserving and, for apply_fix_all, transactional:

• Atomic: the new bytes go to a sibling <file>.<guid>.alcops.tmp in the same directory, then File.Move(temp, path, overwrite: true). A crash leaves either the old or the new file, never a truncated one. The temp is deleted in finally; a stray one left by a hard kill is swept on the next project load (and on refresh, if older than 30 s).
• Encoding-preserving: the file is read as bytes for the existing stale check, decoded with BOM detection, and written back in the encoding found: UTF-8 with or without BOM, UTF-16 LE/BE, UTF-32. Line endings round-trip. A BOM-less file that is not valid UTF-8 (e.g. Windows-1252, which the compiler accepts via its codepage fallback) is refused with UnsupportedEncoding instead of being re-encoded with U+FFFD, which is what main did.
• Batch rollback: apply_fix_all stages every file first (changed/deleted/unreadable/non-UTF-8 files drop out as conflicts, the rest proceed, as in fix: refresh cached project sessions from disk and guard fix writes against stale content #37), then commits them one by one. If a commit throws, every file already written in that call is restored from the original bytes held in memory and the whole committed set is reported in conflicts with applied: false. A rollback that itself fails, or a file edited after this call wrote it, is named in message and stays in filesChanged.
• conflicts entries gain a kind (StaleFile, UnsupportedEncoding, ReadFailed, WriteFailed, RolledBack, NotWritten); apply_fix returns that kind as error. Additive to the response shape; One error envelope and a fixed error vocabulary for all native tools #41 may rename the vocabulary.

Design notes

Two deliberate deviations from the issue text:

• No .alcops.bak, no File.Replace. Verified in the NAV SDK source: almcp's ProjectWatcher (Filter = "*.al", Renamed handler checks EndsWith(".al")) treats a move-over as an in-place update that keeps the DocumentId, whereas File.Replace with a backup produces two renames, so the watcher removes the document and re-adds it under a new DocumentId, with the document briefly missing. A backup file would also sit in the user's source tree for the whole batch. Rollback therefore works from the original bytes kept in memory, through the same temp + move path.
• The encoding is not stored on TrackedDocument or SourceText. The writer has to read the file for the stale check anyway; the encoding of that read is what gets written back. That is always correct (an editor flipping the BOM between load and write is honoured), needs no plumbing through the fix pipeline, and does not depend on every code-fix provider propagating SourceText.Encoding (ChangedText does, SourceText.From(string) does not).

Other facts established while planning: no Microsoft tool writes .al files (the LSP returns WorkspaceEdits to VS Code; al_writetranslation writes XLIFF in place), and the compiler accepts UTF-8 with or without BOM and UTF-16/32 with BOM, so preserving any detected BOM is safe. Neither our *.al enumeration nor almcp's Path.GetExtension == ".al" check matches the temp name.

Accepted limitations (documented in GuardedFileWriter remarks / AGENTS.md): the stale-check → move window is still best-effort against a concurrent writer (out of scope per the issue); File.Move replaces a symlink at the target path with a regular file and does not preserve Unix mode bits or NTFS ACLs; the temp is not written with WriteThrough, so a power loss immediately after the rename can still lose the new content on some file systems (optional hardening, not done).

Review notes

Three Sonnet /code-review high rounds ran before this PR was opened; the last one found no blocking defect.

Round 1 (fixed in fix: address review round 1):

• Windows-1252 / invalid-UTF-8 files were re-encoded with U+FFFD (pre-existing, now refused as UnsupportedEncoding).
• A read error (sharing violation, access denied) in the staging phase aborted the whole apply_fix_all instead of skipping that file (regression vs fix: refresh cached project sessions from disk and guard fix writes against stale content #37, fixed: ReadFailed).
• Rollback could overwrite an edit the user made between the commit and the failure (fixed: rollback compares current bytes with what this call wrote and leaves an edited file alone).

Carried, not fixed:

• Symlinked .al files are replaced by regular files on write (documented).
• The refresh sweep is a second metadata-only directory walk per tool call; measured as milliseconds, accepted.

Round 2 (fixed in fix: address review round 2):

• The strict decoder only applied to BOM-less files; StreamReader swapped in its lossy encoding when a BOM was present, so a BOM'd file with one invalid byte was still re-encoded as U+FFFD. Decoding now sniffs the BOM itself and uses a strict decoder for every case (UTF-32 LE/BE, UTF-8 BOM, UTF-16 LE/BE, UTF-8 without BOM).

Observed outside this diff (pre-existing, not changed here):

• CodeFixRunner.ApplyFixAllAsync takes OriginalContent from the live workspace rather than the snapshot the fix was computed from. If a concurrent call refreshes the workspace with an external edit while a fix-all is running, the stale check compares against the newer text and the fix computed from the older text can overwrite the edit. Candidate for a follow-up.

Round 3 (fixed in fix: address review round 3, all low severity):

• The commit-phase catch only rolled back on I/O and cancellation exceptions; any other exception type could leave earlier files written. It now rolls back on every exception.
• A fixed text that cannot be encoded in the file's encoding (lone surrogate) aborted the whole batch instead of becoming a per-file UnsupportedEncoding conflict.
• Wording still described the refusal as "not valid UTF-8" in a few places after round 2 widened it to any detected encoding.
• The "Re-run get_fixes and apply_fix." hint was appended to the deleted-file conflict too; it now lives only on the changed-on-disk message.

Accepted as designed (round 3): LoadProjectAsync sweeps with no age guard (a second server instance on the same folder mid-write is out of scope); the refresh sweep is a second directory walk; a commit failure in single-file apply_fix surfaces as { error: "IOException" } via the outer catch rather than a WriteFailed conflict (#41 unifies the vocabulary).

Test plan

• dotnet test --configuration Release: 257 → 293 tests, all passing locally with BC DevTools installed (the almcp-backed tests ran rather than skipped).
• New unit tests on GuardedFileWriter: UTF-8 BOM / no BOM / UTF-16 LE+BE / UTF-32 LE+BE byte-exact round-trips; CRLF and LF; BOM never causes a false StaleFile; no temp left after success; move failure leaves the target untouched and no temp; batch all-fresh / one-stale / second-of-three-fails-rolls-back-first (first file carries a BOM to prove byte-level restore) / rollback-fails-reported / stale-and-failure; invalid UTF-8 refused (single and in a batch); locked file → ReadFailed (Windows-only); edited-after-commit not rolled back; conflict kinds on rollback.
• Tool tests: apply_fix keeps a BOM, returns IOException with the file intact on a move failure, returns UnsupportedEncoding on a non-UTF-8 file; apply_fix_all mixed encodings per file, second commit fails → all three restored and reported with kinds, no temp files left.
• Loader/session tests: stray *.alcops.tmp swept on load and on refresh (age guard respected, .alpackages untouched), temp siblings never enumerated as .al.
• BOM and non-UTF-8 fixtures are created at test time by rewriting bytes; nothing with a BOM is committed.

Fixes #40

🤖 Generated with Claude Code