fix(core): preserve read-only write failures during Windows cleanup

[kevin-lozada-santos]

Fixes #216.

A failed overwrite of a read-only destination on Windows can also fail to unlink the temporary file whose mode was copied from that destination. Cleanup then strands the temporary file and masks the original replacement exception.

This change retries deletion after making only that temporary file writable on Windows. Cleanup errors are logged and the original write exception is re-raised. The successful-write path and pre-rename mode-copy order stay unchanged; the destination is never made writable.

The native Windows regression uses the real os.replace, captures its exception, and asserts that the exact same object is raised to the caller. It checks unchanged destination content and mode, preservation of an unrelated read-only temporary-file sentinel, and removal of only the write's temporary file. Additional tests cover exact UTF-8 bytes and mode for writable overwrites, plus cleanup failures that must be logged without masking the original error.

Validation on native Windows / CPython 3.11.15:

• New regression file on unchanged upstream d4379d87d7f13f041ef3ddeca18cab2751ee471b: 3 failed, 1 passed.
• Same file on this commit: 4 passed.
• Focused git-tools, UTF-8, executor, fact-ID, and issue-reference checks: 118 passed, 3 xfailed, 1 failed. The failure, test_atomic_write_directory_fsync_failure_propagates_and_cleans_temp, also fails on unchanged upstream: it expects a directory fsync error while Windows skips directory fsync. No broader-suite green claim is intended.
• ruff check palinode/ tests/ scripts/: passed.
• bandit -r palinode/ -ll: passed.

Full suite not rerun; unrelated native process-liveness tests remain outside this case. Changelog entry is under the existing Unreleased / Fixed heading.

Commands for reproduction:

python -m pytest tests/test_git_tools_write_cleanup.py -q --tb=short
python -m pytest tests/test_git_tools.py tests/test_git_tools_write_cleanup.py tests/test_utf8_encoding_guard.py tests/test_executor.py tests/test_executor_ops_coverage.py tests/test_executor_replace_guard.py tests/test_fact_ids.py tests/test_no_issue_refs_user_surface.py -q --tb=short

[Paul-Kyle]

Merged — thank you. This is the scope you proposed on #169, built exactly as described.

The test is the part I want to name. Asserting caught.value is replace_errors[0] — the same exception object, not a message match — is the assertion that actually pins "the original failure survives cleanup", and it sidesteps the trap that os.replace legitimately names both the temp source and the destination. Checking filename2 as an attribute rather than parsing the string is the same instinct.

The read-only sentinel is better than the obvious test. "No .tmp remains" would pass for a fix that swept the directory; proving an unrelated read-only temp file survives with its mode intact is what shows the blast radius is one file.

One thing worth having on the record: on POSIX the re-raise inside the inner handler now lands in the outer except OSError and gets logged, so an unlink failure there stops masking the original error too. If that was deliberate, good — it is the same bug in the quieter platform.

And thank you for how you reported the validation. You named the platform and interpreter, scoped the claim to the checks you actually ran, and flagged that test_atomic_write_directory_fsync_failure_propagates_and_cleans_temp fails on unchanged upstream too rather than letting it read as something you broke — then said plainly that no broader-suite green claim was intended. Our CI has no Windows lane, so on Windows changes that report is the evidence, and a report that marks its own edges is worth more than a bigger number.