Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ can never drift.

### Fixed

- CLI `--no-overwrite` also refuses images, sidecars, manifests, and skipped reports that
another process creates while a run is rendering, instead of replacing them at write time.
- Preserve existing files during multi-size and carousel exports by choosing available names and refusing race-created destinations.
- Live-file Reload catches a save that lands while the file is being read, without losing a later local draft.
- Reject observed file changes during bounded reads, including same-length saves, instead of accepting mixed text.
Expand Down
30 changes: 30 additions & 0 deletions RenderingTests/NonReplacingFilePublisherTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import Foundation
import Testing

@testable import VitrineRendering

@Suite("Shared publication failure ownership")
struct NonReplacingFilePublisherTests {
enum Failure: Error, Equatable { case write }

@Test func fallbackFailureDoesNotDeleteAReplacementOwnedByAnotherProducer() throws {
let directory = FileManager.default.temporaryDirectory
.appendingPathComponent("publication-failure-\(UUID().uuidString)")
try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
defer { try? FileManager.default.removeItem(at: directory) }
let output = directory.appendingPathComponent("output.png")
let winner = Data("replacement owned by another producer".utf8)

#expect(throws: Failure.write) {
try NonReplacingFilePublisher.publishByExclusiveCreate(Data("new".utf8), to: output) {
handle, bytes in
try handle.write(contentsOf: bytes.prefix(1))
try FileManager.default.removeItem(at: output)
try winner.write(to: output)
throw Failure.write
}
}

#expect(try Data(contentsOf: output) == winner)
}
}
131 changes: 131 additions & 0 deletions Tests/CLI/CLINoClobberPublicationTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import CoreGraphics
import Foundation
import Testing
import VitrineDomain
import VitrineRendering

@testable import VitrineCLICore

@Suite("CLI no-clobber applies at publication")
struct CLINoClobberPublicationTests: CLITestSupport {
@Test(arguments: ["png", "txt", "md", "html"])
func postPreflightWinnersSurviveRealRendering(extensionName: String) throws {
let directory = try makeTempDirectory()
defer { try? FileManager.default.removeItem(at: directory) }
let output = directory.appendingPathComponent("card.png")
let winner = directory.appendingPathComponent("card.\(extensionName)")
let original = Data("another producer's completed artifact".utf8)
let options = try CLIArguments.parse([
"render", "snippet.swift", "--out", output.path,
"--no-overwrite", "--sidecars", "all",
])
try CLIOutputWriter.guardNoOverwriteTargetsAvailable(beside: output, options: options)
// The other producer commits after preflight, before our real render/write path.
try original.write(to: winner)
var config = SnapshotConfig()
config.code = "let value = 42"
config.language = .swift

#expect(throws: CLIError.writeFailed(path: winner.path)) {
_ = try CLIOutputWriter.renderAndWriteArtifact(config, options: options, to: output)
}
#expect(try Data(contentsOf: winner) == original)
if extensionName != "png" {
#expect(try decodePNG(at: output.path).width > 0)
}
}

@Test(arguments: ["--manifest", "--skipped-report"])
func batchReportsPreserveWinnersCreatedAfterRendering(flag: String) throws {
let root = try makeTempDirectory()
defer { try? FileManager.default.removeItem(at: root) }
let input = root.appendingPathComponent("input", isDirectory: true)
let output = root.appendingPathComponent("output", isDirectory: true)
let report = root.appendingPathComponent("report.json")
try FileManager.default.createDirectory(at: input, withIntermediateDirectories: true)
_ = try writeInput("let value = 42", named: "Card.swift", in: input)
let options = try CLIArguments.parse([
"batch", input.path, "--out", output.path, "--no-overwrite", flag, report.path,
])
let original = Data("another producer's report".utf8)
var reachedPublication = false

#expect(throws: CLIError.writeFailed(path: report.path)) {
try CLIBatchRenderer.run(
options,
beforeReports: {
reachedPublication = true
try original.write(to: report)
})
}

#expect(reachedPublication)
#expect(try Data(contentsOf: report) == original)
#expect(try decodePNG(at: output.appendingPathComponent("Card.png").path).width > 0)
}

@Test func lateDanglingSymlinkIsPreservedAndNeverFollowed() throws {
let directory = try makeTempDirectory()
defer { try? FileManager.default.removeItem(at: directory) }
let output = directory.appendingPathComponent("manifest.json")
let target = directory.appendingPathComponent("other.json")
try FileManager.default.createSymbolicLink(at: output, withDestinationURL: target)

#expect(throws: CLIError.writeFailed(path: output.path)) {
try CLIOutputWriter.write(Data("new".utf8), to: output, noOverwrite: true)
}
#expect(
try FileManager.default.destinationOfSymbolicLink(atPath: output.path) == target.path)
#expect(!FileManager.default.fileExists(atPath: target.path))
}

@Test(arguments: ["png", "txt"])
func preflightReportsADanglingSymlinkBeforeRendering(extensionName: String) throws {
let directory = try makeTempDirectory()
defer { try? FileManager.default.removeItem(at: directory) }
let output = directory.appendingPathComponent("card.png")
let occupied = directory.appendingPathComponent("card.\(extensionName)")
try FileManager.default.createSymbolicLink(
at: occupied, withDestinationURL: directory.appendingPathComponent("missing"))
let options = try CLIArguments.parse([
"render", "snippet.swift", "--out", output.path, "--no-overwrite", "--sidecars",
"text",
])

#expect(throws: CLIError.outputExists(path: occupied.path)) {
try CLIOutputWriter.guardNoOverwriteTargetsAvailable(beside: output, options: options)
}
}

@Test(arguments: ["--manifest", "--skipped-report"])
func batchPreflightReportsADanglingSymlinkReport(flag: String) throws {
let root = try makeTempDirectory()
defer { try? FileManager.default.removeItem(at: root) }
let input = root.appendingPathComponent("input", isDirectory: true)
let output = root.appendingPathComponent("output", isDirectory: true)
let report = root.appendingPathComponent("report.json")
try FileManager.default.createDirectory(at: input, withIntermediateDirectories: true)
_ = try writeInput("let value = 42", named: "Card.swift", in: input)
try FileManager.default.createSymbolicLink(
at: report, withDestinationURL: root.appendingPathComponent("missing.json"))
let options = try CLIArguments.parse([
"batch", input.path, "--out", output.path, "--no-overwrite", flag, report.path,
])

#expect(throws: CLIError.outputExists(path: report.path)) {
try CLIBatchRenderer.run(options)
}
#expect(!FileManager.default.fileExists(atPath: output.path))
}

@Test func defaultOverwriteModeStillReplacesAnExistingOutput() throws {
let directory = try makeTempDirectory()
defer { try? FileManager.default.removeItem(at: directory) }
let output = directory.appendingPathComponent("report.json")
try Data("old".utf8).write(to: output)

try CLIOutputWriter.write(Data("new".utf8), to: output, noOverwrite: false)

#expect(try Data(contentsOf: output) == Data("new".utf8))
}
}
20 changes: 12 additions & 8 deletions Vitrine/CLI/CLIBatchRenderer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ enum CLIBatchRenderer {
try FileManager.default.contentsOfDirectory(
at: $0, includingPropertiesForKeys: [.isRegularFileKey],
options: [.skipsHiddenFiles])
}
},
beforeReports: (() throws -> Void)? = nil
) throws -> String {
let background = try CLIRenderResources.prepareBackground(options)
defer { background.removeTemporaryFiles() }
Expand Down Expand Up @@ -183,8 +184,11 @@ enum CLIBatchRenderer {
)
let summary =
"\(action) \(rendered) image\(rendered == 1 ? "" : "s") to \(outputDirectory.path)"
try writeSkippedReport(skippedReport, path: options.skippedReportPath)
try writeBatchManifest(manifest, path: options.batchManifestPath)
try beforeReports?()
try writeSkippedReport(
skippedReport, path: options.skippedReportPath, noOverwrite: options.noOverwrite)
try writeBatchManifest(
manifest, path: options.batchManifestPath, noOverwrite: options.noOverwrite)
if rendered == 0, options.failOnEmpty {
throw CLIError.batchEmpty(skipped: skipped)
}
Expand Down Expand Up @@ -259,7 +263,7 @@ enum CLIBatchRenderer {
"The batch report at \"\(report.path)\" conflicts with an input resource "
+ "or another output. Choose distinct paths for resources and outputs.")
}
if options.noOverwrite, FileManager.default.fileExists(atPath: report.path) {
if options.noOverwrite, CLIOutputWriter.entryExists(at: report) {
throw CLIError.outputExists(path: report.path)
}
try guardExistingInputReport(
Expand Down Expand Up @@ -559,7 +563,7 @@ enum CLIBatchRenderer {
/// useful CI artifact (`[]`) because it proves the batch scanned without omissions.
private static func writeSkippedReport(
_ skippedReport: [SkippedReportEntry],
path: String?
path: String?, noOverwrite: Bool
) throws {
guard let path, !path.isEmpty else { return }
let url = URL(fileURLWithPath: path)
Expand All @@ -576,7 +580,7 @@ enum CLIBatchRenderer {
encoded.append(0x0A)
data = encoded
}
try data.write(to: url, options: .atomic)
try CLIOutputWriter.write(data, to: url, noOverwrite: noOverwrite)
} catch {
throw CLIError.writeFailed(path: path)
}
Expand All @@ -586,7 +590,7 @@ enum CLIBatchRenderer {
/// for CI because it proves discovery completed even when no inputs matched.
private static func writeBatchManifest(
_ manifest: [BatchManifestEntry],
path: String?
path: String?, noOverwrite: Bool
) throws {
guard let path, !path.isEmpty else { return }
let url = URL(fileURLWithPath: path)
Expand All @@ -603,7 +607,7 @@ enum CLIBatchRenderer {
encoded.append(0x0A)
data = encoded
}
try data.write(to: url, options: .atomic)
try CLIOutputWriter.write(data, to: url, noOverwrite: noOverwrite)
} catch {
throw CLIError.writeFailed(path: path)
}
Expand Down
63 changes: 26 additions & 37 deletions Vitrine/CLI/CLIOutputWriter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,14 @@ enum CLIOutputWriter {
/// Finds the first existing output target when `--no-overwrite` is active.
static func existingNoOverwriteTarget(beside imageURL: URL, options: CLIOptions) -> URL? {
guard options.noOverwrite else { return nil }
return outputTargets(beside: imageURL, options: options).first {
FileManager.default.fileExists(atPath: $0.path)
}
return outputTargets(beside: imageURL, options: options).first(where: entryExists(at:))
}

/// Whether any directory entry occupies `url`, without following a final symlink.
/// Publication refuses a dangling symlink too, so preflight must report it before
/// rendering rather than letting the run fail late with a generic write error.
static func entryExists(at url: URL) -> Bool {
(try? FileManager.default.attributesOfItem(atPath: url.path)) != nil
}

/// Enforces `--no-overwrite` before rendering/copying so a run fails without
Expand Down Expand Up @@ -171,7 +176,7 @@ enum CLIOutputWriter {
} catch let error {
throw CLIError.renderFailure(error)
}
try write(payload.data, to: url)
try write(payload.data, to: url, noOverwrite: options.noOverwrite)
if options.textSidecar { try writeTextSidecar(for: config, options: options, beside: url) }
if options.markdownSidecar {
try writeMarkdownSidecar(for: config, options: options, beside: url)
Expand Down Expand Up @@ -210,15 +215,9 @@ enum CLIOutputWriter {
for config: SnapshotConfig, options: CLIOptions, beside imageURL: URL
) throws {
let sidecarURL = imageURL.deletingPathExtension().appendingPathExtension("txt")
do {
try Data(config.sidecarText.utf8).write(to: sidecarURL, options: .atomic)
} catch {
let nsError = error as NSError
Log.export.error(
"CLI text-sidecar write failed (\(nsError.domain, privacy: .public) \(nsError.code, privacy: .public))"
)
throw CLIError.writeFailed(path: sidecarURL.path)
}
// `write` already logs the underlying error and maps it to `writeFailed`.
try write(
Data(config.sidecarText.utf8), to: sidecarURL, noOverwrite: options.noOverwrite)
}

/// Writes the Markdown sidecar next to the rendered image at `imageURL`
Expand All @@ -232,15 +231,8 @@ enum CLIOutputWriter {
let sidecarURL = imageURL.deletingPathExtension().appendingPathExtension("md")
let contents = markdownSidecarContents(
for: config, imageName: imageURL.lastPathComponent)
do {
try Data(contents.utf8).write(to: sidecarURL, options: .atomic)
} catch {
let nsError = error as NSError
Log.export.error(
"CLI markdown-sidecar write failed (\(nsError.domain, privacy: .public) \(nsError.code, privacy: .public))"
)
throw CLIError.writeFailed(path: sidecarURL.path)
}
// `write` already logs the underlying error and maps it to `writeFailed`.
try write(Data(contents.utf8), to: sidecarURL, noOverwrite: options.noOverwrite)
}

/// Writes the HTML sidecar next to the rendered image at `imageURL`
Expand All @@ -252,15 +244,8 @@ enum CLIOutputWriter {
) throws {
let sidecarURL = imageURL.deletingPathExtension().appendingPathExtension("html")
let contents = htmlSidecarContents(for: config, imageName: imageURL.lastPathComponent)
do {
try Data(contents.utf8).write(to: sidecarURL, options: .atomic)
} catch {
let nsError = error as NSError
Log.export.error(
"CLI html-sidecar write failed (\(nsError.domain, privacy: .public) \(nsError.code, privacy: .public))"
)
throw CLIError.writeFailed(path: sidecarURL.path)
}
// `write` already logs the underlying error and maps it to `writeFailed`.
try write(Data(contents.utf8), to: sidecarURL, noOverwrite: options.noOverwrite)
}

/// Builds the Markdown sidecar body: `![alt](image)` + a fenced code block.
Expand All @@ -284,13 +269,17 @@ enum CLIOutputWriter {

/// Writes `data` to `url`, mapping any I/O failure to `CLIError.writeFailed`.
///
/// Atomic, like every other write the batch renderer and the app's exchange files
/// perform: an interrupted run (a full disk, or Ctrl-C between two `write` calls)
/// must not leave a truncated image or sidecar that looks complete to whatever
/// script consumes it.
private static func write(_ data: Data, to url: URL) throws {
/// No-clobber is enforced at publication, including a destination created after
/// preflight. Each file commits independently; a later sidecar failure preserves
/// earlier completed outputs and reports failure. The shared fallback on volumes
/// without exclusive rename can leave a partial file on I/O failure.
static func write(_ data: Data, to url: URL, noOverwrite: Bool) throws {
do {
try data.write(to: url, options: .atomic)
if noOverwrite {
try NonReplacingFilePublisher.publish(data, to: url)
} else {
try data.write(to: url, options: .atomic)
}
} catch {
// Log only the format, never the (user-chosen) path (privacy policy).
let nsError = error as NSError
Expand Down
Loading
Loading