[#188] Add partial-cleanup on write failure in PackWriter

[bguidolim]

Summary

• PackWriter.write() now cleans up the partial output directory if any step fails after directory creation, preventing users from hitting ExportError.outputDirectoryExists on retry
• Write logic extracted into private writeContents helper; the public method wraps it in a do/catch with best-effort try? fm.removeItem cleanup
• Added PackWriterTests with 3 tests: happy-path file creation, cleanup-on-failure verification, and pre-existing directory error

Test plan

• swift test --filter MCSTests.PackWriterTests — all 3 tests pass
• Manual: run mcs export <dir> with a source file removed mid-write to verify cleanup

Closes #188

[Copilot]

Pull request overview

This PR improves PackWriter.write() reliability during export by cleaning up partially-written output directories when a write fails, preventing retry failures due to an existing output directory.

Changes:

• Wrapped pack writing in do/catch to best-effort remove the output directory on failure.
• Extracted the multi-step write logic into a private writeContents helper.
• Added PackWriterTests covering happy path, cleanup-on-failure, and pre-existing directory error behavior.

Reviewed changes

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

File	Description
Sources/mcs/Export/PackWriter.swift	Adds failure cleanup behavior and refactors write steps into a helper.
Tests/MCSTests/PackWriterTests.swift	Adds regression tests for successful write, cleanup on failure, and existing output directory handling.

---

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

[Copilot]

write() checks fileExists and then writeContents creates the output directory using createDirectory(..., withIntermediateDirectories: true), which will not fail if the directory already exists. If the directory is created concurrently between the check and createDirectory, this code can end up writing into an unexpected pre-existing directory and, on later failure, delete that directory in the catch cleanup. Consider creating outputDir in a way that fails if it already exists (e.g., create parent dirs separately, then createDirectory with withIntermediateDirectories: false and map EEXIST to ExportError.outputDirectoryExists), or writing to a uniquely-named temp dir and renaming into place once complete.

Suggested change

	if fm.fileExists(atPath: outputDir.path) {
	throw ExportError.outputDirectoryExists(outputDir.path)
	}
	do {
	try writeContents(result: result, to: outputDir, fm: fm)
	} catch {
	try? fm.removeItem(at: outputDir)
	throw error
	}
	}
	private func writeContents(result: ManifestBuilder.BuildResult, to outputDir: URL, fm: FileManager) throws {
	try fm.createDirectory(at: outputDir, withIntermediateDirectories: true)
	// First, check if the output directory already exists.
	if fm.fileExists(atPath: outputDir.path) {
	throw ExportError.outputDirectoryExists(outputDir.path)
	}
	// Ensure the parent directory exists.
	let parentDir = outputDir.deletingLastPathComponent()
	try fm.createDirectory(at: parentDir, withIntermediateDirectories: true)
	// Create the output directory itself, failing if it already exists.
	do {
	try fm.createDirectory(at: outputDir, withIntermediateDirectories: false)
	} catch {
	// Map "already exists" errors to ExportError.outputDirectoryExists.
	if let cocoaError = error as? CocoaError, cocoaError.code == .fileWriteFileExists {
	throw ExportError.outputDirectoryExists(outputDir.path)
	}
	if let posixError = error as? POSIXError, posixError.code == .EEXIST {
	throw ExportError.outputDirectoryExists(outputDir.path)
	}
	throw error
	}
	do {
	try writeContents(result: result, to: outputDir, fm: fm)
	} catch {
	// Only remove the directory we created if a later step fails.
	try? fm.removeItem(at: outputDir)
	throw error
	}
	}
	private func writeContents(result: ManifestBuilder.BuildResult, to outputDir: URL, fm: FileManager) throws {

[Copilot]

The doc comment says the method "Cleans up the partial directory" but the implementation uses try? fm.removeItem(...) and discards cleanup failures. Either adjust the comment to clarify this is best-effort cleanup, or emit a warning to output when removal fails so users understand why a retry may still hit outputDirectoryExists.