Fix: orphaned temp files in BinaryHandler.Dirty cause patch failures on repeated update runs

[Copilot]

Each patch application wrote the result to a randomly-named temp file (e.g., 4nl40ro2.p13_app.exe), copied it back to the original path, but never deleted the temp file. On the next update run, GetAllFiles picked up these orphans alongside the already-updated originals, causing the old→new patch to be re-applied to an already-new file — producing corrupt output or a runtime error.

Changes

• BinaryHandler.Dirty: Add File.Delete(_newfilePath) after the copy-back, cleaning up the temp file immediately after its content is committed to the final path.

// Before
File.Copy(_newfilePath, _oldfilePath, true);
// _newfilePath left behind as orphan

// After
File.Copy(_newfilePath, _oldfilePath, true);
File.Delete(_newfilePath); // temp file removed

• DifferentialCoreTests: Add regression test Dirty_WithPatchFiles_DoesNotLeaveTemporaryFiles asserting that the app directory contains exactly the expected files (no orphan temp files) after a patch run.

[JusterZhu]

@copilot The description in the issue indicates that the update failed because the patch was repeatedly restored to the same file. Could you explain why you deleted the file in this way?

[Copilot]

@copilot The description in the issue indicates that the update failed because the patch was repeatedly restored to the same file. Could you explain why you deleted the file in this way?

Here is the full call chain to explain the deletion:

1. DefaultDirtyStrategy.ApplyPatch (line 82–85) creates a randomly-named temp file in the same app directory as the file being patched:

   newPath = /app/xyz123_test.exe   ← temp output buffer
   BinaryHandler.Dirty(appFilePath=/app/test.exe, newPath, patchFilePath)
   

2. BinaryHandler.Dirty reads from _oldfilePath (/app/test.exe) and writes the patched result to _newfilePath (/app/xyz123_test.exe). It can't write to the original while reading from it.

3. After patching, the code deletes the original and copies the temp file over it:

   File.Delete(_oldfilePath);         // delete /app/test.exe
   File.Copy(_newfilePath, _oldfilePath, true); // copy xyz123_test.exe → test.exe
   // _newfilePath (/app/xyz123_test.exe) was never deleted ← BUG

4. On the second run, GetAllFiles(appPath) enumerates both test.exe (already at the new version) and xyz123_test.exe (orphan). The DefaultDirtyMatcher matches test.exe against test.exe.patch again and tries to apply an old→new patch to a file that is already at the new version, which corrupts the output or throws an exception — exactly the "update interrupted" error described in the issue.

The fix adds File.Delete(_newfilePath) immediately after the copy-back, ensuring the temp buffer is removed once its content is safely committed to the final path. File.Copy + File.Delete is used (instead of File.Move) because the library targets netstandard2.0 where the overwrite-capable File.Move(src, dst, true) is not available.

[JusterZhu]

@copilot As of now, will all the code changes introduce new issues or affect the execution logic of the original code?

[Copilot]

@copilot As of now, will all the code changes introduce new issues or affect the execution logic of the original code?

The changes are minimal and do not alter any existing execution logic:

BinaryHandler.cs — one line added (line 376)

if (File.Exists(_newfilePath))
{
    File.SetAttributes(_newfilePath, FileAttributes.Normal);
    File.Copy(_newfilePath, _oldfilePath, true);  // ← unchanged
    File.Delete(_newfilePath);                    // ← new line
}

• The new File.Delete is inside the existing File.Exists guard, so it only runs when the temp file is actually present — no risk of a FileNotFoundException.
• The copy to _oldfilePath completes fully before the delete, so the updated application file is always committed first. If the copy throws, the delete is never reached (the temp file stays, which is safe).
• File.SetAttributes was already called before File.Copy, so the file is already writable/normal before deletion.
• No other call site creates or depends on _newfilePath after this point; the Task.Run lambda ends immediately after.

DifferentialCoreTests.cs — new regression test only
The test is entirely additive: it creates its own isolated temp directories (via the existing _appDirectory / _patchDirectory fixtures), exercises DifferentialCore.Dirty, and asserts that no extra files remain. No existing test is modified.

No execution path for the normal (non-error) case changes; the only behavioral difference is that the temp file is now cleaned up.

[JusterZhu]

@copilot I'm just asking. Please don't modify the code yet. Why isn't there a check during the update to determine whether the hash value of each file has been differentially updated?

[Copilot]

@copilot I'm just asking. Please don't modify the code yet. Why isn't there a check during the update to determine whether the hash value of each file has been differentially updated?

The Clean (patch-generation) phase already does check hashes: DefaultCleanStrategy calls StorageManager.HashEquals(oldFile, newFile) before generating a patch and skips files whose content is identical. So the patch package only ever contains patches for files that genuinely changed.

The Dirty (patch-application) phase does not repeat that check. It relies on a different idempotency mechanism: at the end of DefaultDirtyStrategy.CopyUnknownFiles, StorageManager.DeleteDirectory(patchPath) deletes the entire patch directory. Once the patches have been applied and the patch directory is gone, a second run finds no patch files to match — so nothing is re-applied.

The bug was entirely in the temp-file lifetime: the randomly-named temp file (e.g., xyz123_test.exe) was created inside the app directory and never deleted. On the next update, GetAllFiles(appPath) enumerated it alongside the real application files. DefaultDirtyMatcher matched the already-updated test.exe against a fresh test.exe.patch from the new package, which then tried to apply an old→new delta to a file that was already at the new version.

Adding a hash-based pre-check in the Dirty phase would be an alternative, but it would require the patch package to carry a manifest of expected post-patch hashes (the current Clean phase does not produce one). The simpler fix — deleting the temp file immediately after the copy-back — closes the root cause without requiring any new data in the patch package.

[Copilot]

Pull request overview

Fixes a differential-update failure mode caused by BinaryHandler.Dirty leaving behind randomly-named temporary “new file” artifacts in the application directory after patch application, and adds a regression test to prevent recurrence.

Changes:

• Delete the temporary patched output file after copying it back to the original path in BinaryHandler.Dirty.
• Add a regression test asserting DifferentialCore.Dirty leaves no extra files in the app directory after applying a patch.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/c#/GeneralUpdate.Differential/Binary/BinaryHandler.cs	Cleans up the temp “new file” after copy-back to prevent orphan artifacts.
src/c#/DifferentialTest/DifferentialCoreTests.cs	Adds regression coverage ensuring no temp files remain after a patch run.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.