Stop a failed save from destroying the document it was saving

[andiwand]

saveSync opened the user's own document and copied into it in place. The window between "destination truncated" and "new content written" was one where any throw left them with a mangled file and a "save failed" toast.

One of the 1★ reviews in the console is exactly that: "tried to save, and got an 'Internal Server Error' that destroyed the file. Good thing it was a copy." Another: "it's going to make you save a new document only to discover it saved none of your changes."

Four problems, in rising order of how often they bite

1. Mode "w" is not required to truncate. That's what "wt" is for. Saving a document shorter than the one already there left the tail of the old file behind the new content — and for a zip container like odt or docx, trailing bytes after the central directory are what make it stop opening. This needs no failure at all to happen, which makes it the likely everyday corruptor rather than the dramatic one.

A provider that rejects the mode outright falls back to "w", so this cannot make saving fail anywhere it works today.

2. close() was not in a finally. Any throw during the copy leaked the descriptor and left flushing undefined. StreamUtil.copy explicitly documents "Closes the input, never the output — the caller owns that one." It's a use block now.

3. The retranslated temp file was only deleted on success, so a failed save leaked it. Now deleted in a finally — and only that one: the other branch hands back the cache file of the document that is still open, which must survive.

4. Nothing was kept, so a half-written destination could not be undone. The current content is now copied into the cache before the write and put back if the write throws. If the rollback itself fails, that copy is deliberately left behind rather than deleted — at that point it is the only copy of the user's document that exists.

Also

onSaveError is posted to the main handler, which onSaveSuccess already was. The listener touches fragment state and calls requireActivity(), neither of which belongs on the loader's background thread. (SnackbarHelper defends itself with runOnUiThread, so this was not visibly crashing — it was just wrong.)

On testing — worth being straight about

None of this is meaningfully covered, and I could not find an honest way to cover it in this repo.

• The truncation fix cannot be demonstrated here. The only content provider available to a test is the app's own FileProvider, and FileProvider.modeToMode maps "w" to MODE_TRUNCATE anyway — so a test would pass identically with and without the fix. The providers that don't truncate are third-party DocumentsProviders (Drive, Proton, various file managers), which a test cannot stand up.
• The rollback path needs a write that fails partway. A destination that fails to open never gets truncated, so it exercises nothing; making a real provider stream throw mid-copy isn't arrangeable without introducing a seam over ContentResolver.
• Saving at all goes through ACTION_CREATE_DOCUMENT, a system picker, which is why the existing instrumented tests cover entering edit mode but stop before the save.

What I did verify: spotlessCheck, assembleLiteDebug, assembleProDebug, lintProDebug, testProDebugUnitTest, testLiteDebugUnitTest all pass. The rest is reviewed by reading. Happy to add the ContentResolver seam and unit-test the rollback with fakes if you'd rather have the coverage than the smaller diff.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43c8244d5d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".