Compare commits

..
Author SHA1 Message Date
gronodandDevin <158243242+devin-ai-integration[bot]@users.noreply.github.com> ef56cdd7d4 Fix M5 regressions and Stage 3 chartread prompt delivery (#50, #52).
Hardens ProcessManager finalisation, ensures stdout/stderr pipe write-ends
close after spawn, sets termination handlers before run(), and fixes the
runCaptured continuation hand-off for fast exits. Resolves the Stage 3
prompt stream being dropped and the handheld fixture UI not advancing,
unskips and repairs the handheld UI test, and fixes averaging panel
accessibility. Includes the selected #52 profile, verification, and
artefact-gating fixes.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-09-09 13:25:39 +01:00
gronod 61c6d62ee2 Merge pull request 'Milestone 5 — Profile generation, verification & installation (#23–#27)' (#51) from feat/23-colprof into milestone/m5-profile 2026-09-09 11:36:35 +01:00
18 changed files with 839 additions and 226 deletions
@@ -87,6 +87,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("targen") let binaryURL = binaryResolver.resolve("targen")
let processId = ProcessID.targen(cleanBasename) let processId = ProcessID.targen(cleanBasename)
await ensureNotRunning(id: processId)
let events = processManager.events() let events = processManager.events()
try await processManager.runStreaming( try await processManager.runStreaming(
id: processId, id: processId,
@@ -121,6 +122,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("printtarg") let binaryURL = binaryResolver.resolve("printtarg")
let processId = ProcessID.printtarg(cleanBasename) let processId = ProcessID.printtarg(cleanBasename)
await ensureNotRunning(id: processId)
let events = processManager.events() let events = processManager.events()
try await processManager.runStreaming( try await processManager.runStreaming(
id: processId, id: processId,
@@ -169,6 +171,19 @@ public struct ArgyllRunner: Sendable {
// MARK: - Shared collection // MARK: - Shared collection
/// Cancels any previous child with the same id and waits for it to
/// finalize, so `runStreaming` / `runCaptured` never sees a
/// `duplicateID` from a leftover process (#50, #52).
private func ensureNotRunning(id: String) async {
guard await processManager.isRunning(id) else { return }
await processManager.kill(id: id)
var attempts = 0
while await processManager.isRunning(id), attempts < 30 {
try? await Task.sleep(for: .milliseconds(100))
attempts += 1
}
}
private struct CollectedRun { private struct CollectedRun {
var exitCode: Int32? var exitCode: Int32?
var stdout: String var stdout: String
@@ -179,10 +194,15 @@ public struct ArgyllRunner: Sendable {
/// Drains the event stream until this child's `exit` event. /// Drains the event stream until this child's `exit` event.
/// stdout is accumulated both per-line (logs) and verbatim (for /// stdout is accumulated both per-line (logs) and verbatim (for
/// the manifest parse the pretty JSON needs its newlines). /// the manifest parse the pretty JSON needs its newlines).
///
/// When `flushPartialLines` is `true`, a background `Task` flushes
/// unterminated output every 500 ms so tools like `colprof` that
/// print dots without newlines still produce log batches.
private func collect( private func collect(
id processId: String, id processId: String,
events: AsyncStream<ProcessEvent>, events: AsyncStream<ProcessEvent>,
onLogBatch: (@Sendable ([String]) -> Void)? onLogBatch: (@Sendable ([String]) -> Void)?,
flushPartialLines: Bool = false
) async -> CollectedRun { ) async -> CollectedRun {
var lines: [String] = [] var lines: [String] = []
var stdout = "" var stdout = ""
@@ -198,6 +218,17 @@ public struct ArgyllRunner: Sendable {
onLogBatch?(out) onLogBatch?(out)
} }
var dotFlushTask: Task<Void, Never>?
if flushPartialLines {
dotFlushTask = Task { [processManager] in
while !Task.isCancelled {
try? await Task.sleep(for: .milliseconds(500))
if Task.isCancelled { break }
await processManager.flushPartialLine(id: processId)
}
}
}
for await event in events { for await event in events {
guard event.id == processId else { continue } guard event.id == processId else { continue }
switch event { switch event {
@@ -229,6 +260,12 @@ public struct ArgyllRunner: Sendable {
break break
} }
} }
dotFlushTask?.cancel()
if let dotFlushTask {
_ = await dotFlushTask.value
}
return CollectedRun(exitCode: exitCode, stdout: stdout, stderr: stderr, lines: lines) return CollectedRun(exitCode: exitCode, stdout: stdout, stderr: stderr, lines: lines)
} }
@@ -242,6 +279,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("instlist") let binaryURL = binaryResolver.resolve("instlist")
let processId = ProcessID.instlist let processId = ProcessID.instlist
await ensureNotRunning(id: processId)
let events = processManager.events() let events = processManager.events()
try await processManager.runStreaming( try await processManager.runStreaming(
id: processId, id: processId,
@@ -301,6 +339,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("average") let binaryURL = binaryResolver.resolve("average")
let processId = ProcessID.average(config.basename) let processId = ProcessID.average(config.basename)
await ensureNotRunning(id: processId)
let events = processManager.events() let events = processManager.events()
try await processManager.runStreaming( try await processManager.runStreaming(
id: processId, id: processId,
@@ -335,6 +374,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("colprof") let binaryURL = binaryResolver.resolve("colprof")
let processId = ProcessID.colprof(cleanBasename) let processId = ProcessID.colprof(cleanBasename)
await ensureNotRunning(id: processId)
let events = processManager.events() let events = processManager.events()
try await processManager.runStreaming( try await processManager.runStreaming(
id: processId, id: processId,
@@ -342,7 +382,12 @@ public struct ArgyllRunner: Sendable {
arguments: args, arguments: args,
workingDirectory: cwd workingDirectory: cwd
) )
let run = await collect(id: processId, events: events, onLogBatch: onLogBatch) let run = await collect(
id: processId,
events: events,
onLogBatch: onLogBatch,
flushPartialLines: true
)
guard run.exitCode == 0 else { guard run.exitCode == 0 else {
throw ArgyllRunnerError.colprofFailed( throw ArgyllRunnerError.colprofFailed(
@@ -368,10 +413,13 @@ public struct ArgyllRunner: Sendable {
/// ///
/// Runs `applycal` captured and performs an in-place replace via /// Runs `applycal` captured and performs an in-place replace via
/// `{input}.applycal.tmp` then `replaceItemAt`. On failure the tmp /// `{input}.applycal.tmp` then `replaceItemAt`. On failure the tmp
/// file is removed and the original is left untouched. /// file is removed and the original is left untouched. The UI must
/// never request `unapply` (#52).
public func runApplycal( public func runApplycal(
config: ApplycalConfig config: ApplycalConfig
) async throws -> URL { ) async throws -> URL {
assert(!config.unapply, "runApplycal does not support unapply")
let inputURL = config.inputProfileURL let inputURL = config.inputProfileURL
let cwd = inputURL.deletingLastPathComponent() let cwd = inputURL.deletingLastPathComponent()
let binaryURL = binaryResolver.resolve("applycal") let binaryURL = binaryResolver.resolve("applycal")
@@ -383,6 +431,8 @@ public struct ArgyllRunner: Sendable {
// Remove any stale tmp from a previous crash. // Remove any stale tmp from a previous crash.
try? fm.removeItem(at: tmpURL) try? fm.removeItem(at: tmpURL)
await ensureNotRunning(id: processId)
let outputConfig = ApplycalConfig( let outputConfig = ApplycalConfig(
calibrationPath: config.calibrationPath, calibrationPath: config.calibrationPath,
inputProfileURL: inputURL, inputProfileURL: inputURL,
@@ -398,8 +448,11 @@ public struct ArgyllRunner: Sendable {
workingDirectory: cwd workingDirectory: cwd
) )
guard result.exitCode == 0 else { guard result.exitCode == 0, !Task.isCancelled else {
try? fm.removeItem(at: tmpURL) try? fm.removeItem(at: tmpURL)
if Task.isCancelled {
throw CancellationError()
}
throw ArgyllRunnerError.applycalFailed( throw ArgyllRunnerError.applycalFailed(
result.stderr.isEmpty result.stderr.isEmpty
? "applycal exited with code \(result.exitCode)" ? "applycal exited with code \(result.exitCode)"
@@ -413,6 +466,15 @@ public struct ArgyllRunner: Sendable {
) )
} }
let attrs = try? fm.attributesOfItem(atPath: tmpURL.path)
let size = attrs?[.size] as? UInt64 ?? 0
guard size >= 128 else {
try? fm.removeItem(at: tmpURL)
throw ArgyllRunnerError.applycalFailed(
"calibrated profile is too small (\(size) bytes)"
)
}
do { do {
if fm.fileExists(atPath: inputURL.path) { if fm.fileExists(atPath: inputURL.path) {
_ = try fm.replaceItemAt(inputURL, withItemAt: tmpURL) _ = try fm.replaceItemAt(inputURL, withItemAt: tmpURL)
@@ -441,6 +503,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("iccgamut") let binaryURL = binaryResolver.resolve("iccgamut")
let processId = ProcessID.iccgamut(stem: stem) let processId = ProcessID.iccgamut(stem: stem)
await ensureNotRunning(id: processId)
let events = processManager.events() let events = processManager.events()
try await processManager.runStreaming( try await processManager.runStreaming(
id: processId, id: processId,
@@ -480,6 +543,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("profcheck") let binaryURL = binaryResolver.resolve("profcheck")
let processId = ProcessID.profcheck(ti3Path: ti3Path) let processId = ProcessID.profcheck(ti3Path: ti3Path)
await ensureNotRunning(id: processId)
let events = processManager.events() let events = processManager.events()
try await processManager.runStreaming( try await processManager.runStreaming(
id: processId, id: processId,
@@ -547,11 +611,21 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("chartread") let binaryURL = binaryResolver.resolve("chartread")
let processId = ProcessID.chartread(cleanBasename) let processId = ProcessID.chartread(cleanBasename)
let processManager = self.processManager let processManager = self.processManager
let isXY = config.isXY
return AsyncStream { continuation in return AsyncStream { continuation in
let task = Task { let task = Task {
await ensureNotRunning(id: processId)
let events = processManager.events() let events = processManager.events()
// Register the XY parking hook before spawning.
await processManager.setPreKillHook(id: processId) { [processManager] in
if isXY {
try? await processManager.sendStdin(id: processId, bytes: ChartreadInput.quit.bytes)
try? await Task.sleep(for: .milliseconds(500))
}
}
do { do {
try await processManager.runStreaming( try await processManager.runStreaming(
id: processId, id: processId,
@@ -582,20 +656,22 @@ public struct ArgyllRunner: Sendable {
switch event { switch event {
case .stdout(_, let line): case .stdout(_, let line):
let classified = ChartreadClassifier.classify(line: line, previousState: state) let previous = state
let classified = ChartreadClassifier.classify(line: line, previousState: previous)
state = classified.state state = classified.state
if classified.isRemoveSheetNotice { if classified.isRemoveSheetNotice {
continuation.yield(.removeSheetNotice) continuation.yield(.removeSheetNotice)
} }
if classified.sheetNumber != nil || classified.alignmentPatch != nil {
continuation.yield(.prompt(classified)) let shouldPrompt =
} else if state != previousOrContinuationState(state, classified) { classified.sheetNumber != nil
// Only emit prompt when the state meaningfully changes. || classified.alignmentPatch != nil
continuation.yield(.prompt(classified)) || classified.requestedWarningKey != nil
} else if state == .tablePlaceSheet || state == .tableAlign { || classified.state != previous
// Continuation lines in table states are still prompts. || classified.isTableContinuation
continuation.yield(.prompt(classified))
} else if classified.requestedWarningKey != nil { if shouldPrompt {
continuation.yield(.prompt(classified)) continuation.yield(.prompt(classified))
} }
@@ -632,6 +708,11 @@ public struct ArgyllRunner: Sendable {
} }
} }
if Task.isCancelled {
continuation.finish()
return
}
let canonical = cwd.appendingPathComponent("\(cleanBasename).ti3") let canonical = cwd.appendingPathComponent("\(cleanBasename).ti3")
if let code = exitCode, code == 0 { if let code = exitCode, code == 0 {
if FileManager.default.fileExists(atPath: canonical.path) { if FileManager.default.fileExists(atPath: canonical.path) {
@@ -647,15 +728,13 @@ public struct ArgyllRunner: Sendable {
continuation.onTermination = { _ in continuation.onTermination = { _ in
task.cancel() task.cancel()
Task {
await processManager.kill(id: processId)
}
} }
} }
} }
private func previousOrContinuationState(_ state: ChartreadState, _ classified: ChartreadClassifyResult) -> ChartreadState {
if classified.isTableContinuation { return .promptContinue }
return state
}
/// Send an exact input sequence to the running `chartread` child. /// Send an exact input sequence to the running `chartread` child.
public func sendChartreadInput(basename: String, input: ChartreadInput) async throws { public func sendChartreadInput(basename: String, input: ChartreadInput) async throws {
let cleanBasename = try PathSecurity.sanitizeBasename(basename) let cleanBasename = try PathSecurity.sanitizeBasename(basename)
@@ -665,17 +744,14 @@ public struct ArgyllRunner: Sendable {
/// Terminate a running `chartread` child. /// Terminate a running `chartread` child.
/// ///
/// For XY tables, sends `q\n` first and waits ~500 ms so the head parks. /// The actual XY parking is handled by the pre-kill hook registered in
/// `runChartread`.
public func cancelChartread(basename: String, isXY: Bool = false) { public func cancelChartread(basename: String, isXY: Bool = false) {
let cleanBasename = try? PathSecurity.sanitizeBasename(basename) let cleanBasename = try? PathSecurity.sanitizeBasename(basename)
guard let cleanBasename else { return } guard let cleanBasename else { return }
let processId = ProcessID.chartread(cleanBasename) let processId = ProcessID.chartread(cleanBasename)
Task { Task {
if isXY {
try? await processManager.sendStdin(id: processId, bytes: ChartreadInput.quit.bytes)
try? await Task.sleep(for: .milliseconds(500))
}
await processManager.kill(id: processId) await processManager.kill(id: processId)
} }
} }
@@ -153,7 +153,7 @@ public enum ChartreadClassifier {
let phrases = [ let phrases = [
"'d' if/when done", "d to finish/save", "all strips/patches read", "'d' if/when done", "d to finish/save", "all strips/patches read",
"all strips read", "all patches read", "done reading", "all strips read", "all patches read", "done reading",
"'d' to save", "press d to", "hit 'd'" "'d' to save", "press d to", "hit 'd'", "d to finish", "d to save"
] ]
if phrases.contains(where: { text.contains($0) }) { if phrases.contains(where: { text.contains($0) }) {
return ChartreadClassifyResult(state: .allStripsRead) return ChartreadClassifyResult(state: .allStripsRead)
@@ -163,20 +163,28 @@ public enum ChartreadClassifier {
// 7. Warnings / prompts needing a key. // 7. Warnings / prompts needing a key.
private static func warning(text: String, previous: ChartreadState) -> ChartreadClassifyResult? { private static func warning(text: String, previous: ChartreadState) -> ChartreadClassifyResult? {
let lower = text
let warningSignals = [ let warningSignals = [
"(warning)", "use it anyway", "seem to have read strip pass", "(warning)", "use it anyway", "seem to have read strip",
"unexpected response", "seem to have read", "misread", "unexpected response", "try again", "do you want to",
"try again", "do you want to" "abort ? - are you sure", "are you sure"
] ]
guard warningSignals.contains(where: { text.contains($0) }) else { return nil }
let isWarningPrompt =
warningSignals.contains(where: { lower.contains($0) })
|| lower.contains("(y/n)")
|| lower.contains("'y' or 'n'")
|| lower.contains("?")
guard isWarningPrompt else { return nil }
var key: String? var key: String?
if text.contains("(y/n)") || text.contains("'y' or 'n'") { if lower.contains("(y/n)") || lower.contains("'y' or 'n'") {
// Default to asking the user; no automatic key. // Default to asking the user; no automatic key.
key = nil key = nil
} else if text.contains("'y'") || text.contains("press y") || text.contains("hit 'y'") { } else if lower.contains("'y'") || lower.contains("press y") || lower.contains("hit 'y'") {
key = "y" key = "y"
} else if text.contains("'n'") || text.contains("press n") || text.contains("hit 'n'") { } else if lower.contains("'n'") || lower.contains("press n") || lower.contains("hit 'n'") {
key = "n" key = "n"
} }
@@ -195,12 +203,14 @@ public enum ChartreadClassifier {
!hasLocate !hasLocate
else { return nil } else { return nil }
if lowercased.contains("hit any key to continue") if lowercased.contains("calibrat")
|| lowercased.contains("hit space to continue") || lowercased.contains("white reference")
|| lowercased.contains("calibration")
|| lowercased.contains("calibrate")
|| lowercased.contains("white tile") || lowercased.contains("white tile")
|| lowercased.contains("standard tile") { || lowercased.contains("standard tile")
|| lowercased.contains("reference")
|| lowercased.contains("tile")
|| lowercased.contains("hit any key to continue")
|| lowercased.contains("hit space to continue") {
return ChartreadClassifyResult(state: .calibrating) return ChartreadClassifyResult(state: .calibrating)
} }
return nil return nil
@@ -209,11 +219,29 @@ public enum ChartreadClassifier {
// 9. Awaiting strip. // 9. Awaiting strip.
private static func awaitingStrip(text: String, previous: ChartreadState) -> ChartreadClassifyResult? { private static func awaitingStrip(text: String, previous: ChartreadState) -> ChartreadClassifyResult? {
let lowercased = text.lowercased() let lowercased = text.lowercased()
// These are explicit, multi-word prompts; we deliberately do NOT
// match bare "read strip" so that error lines like
// "failed to read strip" or "error reading strip" fall through to
// the error matcher.
let phrases = [ let phrases = [
"hit ... read ... strip", "ready to read", "read ... strip ... key", "ready to read",
"hit any key to read", "ready to read strip", "hit a key to read", "hit any key to read",
"press any key to read", "read strip" "hit a key to read",
"hit space to read",
"hit [space] to read",
"press any key to read",
"press space to read",
"trigger instrument",
"start reading",
"read next strip"
] ]
// Also permit "hit X to read strip Y" or "ready to read strip Z".
if lowercased.range(of: #"(hit|press).+to\s+read\s+strip"#, options: .regularExpression) != nil {
return ChartreadClassifyResult(state: .awaitingStrip)
}
guard phrases.contains(where: { lowercased.contains($0) }) else { return nil } guard phrases.contains(where: { lowercased.contains($0) }) else { return nil }
return ChartreadClassifyResult(state: .awaitingStrip) return ChartreadClassifyResult(state: .awaitingStrip)
} }
@@ -228,16 +256,27 @@ public enum ChartreadClassifier {
// 11. Error. // 11. Error.
private static func error(text: String, previous: ChartreadState) -> ChartreadClassifyResult? { private static func error(text: String, previous: ChartreadState) -> ChartreadClassifyResult? {
let phrases = ["error", "too fast", "too slow", "misread", "failed to read", "failed"] let lower = text.lowercased()
// Avoid false positives inside harmless words by matching full words where possible.
let lower = text
guard phrases.contains(where: { phrase in
lower.contains(phrase) && !lower.contains("no error")
}) else { return nil }
if lower.contains("misread") || lower.contains("failed to read") || lower.contains("error") { // Avoid false positives from confirmation prompts and "no error" status.
guard !lower.contains("no error") else { return nil }
guard !lower.contains("(y/n)")
&& !lower.contains("'y' or 'n'")
&& !lower.contains("?")
else { return nil }
let phraseMatches = ["failed to read", "error reading", "too fast", "too slow", "misread"]
for phrase in phraseMatches {
if lower.contains(phrase) {
return ChartreadClassifyResult(state: .error)
}
}
// Whole-word "error" only bare "failed" alone is not enough.
if lower.range(of: #"\berror\b"#, options: .regularExpression) != nil {
return ChartreadClassifyResult(state: .error) return ChartreadClassifyResult(state: .error)
} }
return nil return nil
} }
} }
@@ -35,6 +35,18 @@ public struct ProcessLineDecoder: Sendable {
return rest.isEmpty ? nil : Self.decode(rest) return rest.isEmpty ? nil : Self.decode(rest)
} }
/// Emits the current unterminated tail as a single line and clears it.
/// Used by `ProcessManager.flushPartialLine` for tools that emit
/// progress dots without newlines.
public mutating func flushPartial() -> String? {
guard !pending.isEmpty else { return nil }
var rest = pending
pending.removeAll(keepingCapacity: false)
if rest.last == 0x0D { rest = rest.dropLast() }
let text = Self.decode(rest)
return text.isEmpty ? nil : text
}
private static func decode(_ bytes: Data.SubSequence) -> String { private static func decode(_ bytes: Data.SubSequence) -> String {
String(decoding: bytes, as: UTF8.self) String(decoding: bytes, as: UTF8.self)
} }
@@ -20,8 +20,11 @@ public struct CapturedResult: Sendable, Equatable {
/// with the prefix stripped; all other stdout is `stdout` events. /// with the prefix stripped; all other stdout is `stdout` events.
/// - `exit` is emitted exactly once per child, and only after both /// - `exit` is emitted exactly once per child, and only after both
/// output pipes reach EOF so no buffered output is lost on fast /// output pipes reach EOF so no buffered output is lost on fast
/// exits or kills. /// exits or kills. If EOFs never arrive, a watchdog finalizes.
/// - `kill` drops the stdin handle so writers fail fast. /// - `kill` runs a pre-kill hook (e.g. XY `q\n` + 500 ms park) before
/// terminating. Hooks are removed once the child finalizes.
/// - `killAll` on `NSApplication.willTerminate` and last-window close
/// runs all hooks and terminates every child (#147, #149).
public actor ProcessManager { public actor ProcessManager {
public static let rowColorsPrefix = "ROW_COLORS_JSON: " public static let rowColorsPrefix = "ROW_COLORS_JSON: "
@@ -92,12 +95,18 @@ public actor ProcessManager {
/// pipes have also reached EOF. /// pipes have also reached EOF.
var pendingExitCode: Int32? var pendingExitCode: Int32?
var finalized = false var finalized = false
/// Watchdog that forces finalization if EOFs never arrive.
var finalizeTask: Task<Void, Never>?
} }
private var children: [String: RunningChild] = [:] private var children: [String: RunningChild] = [:]
/// Processes owned by `runCaptured` (dup detection + kill support). /// Processes owned by `runCaptured` (dup detection + kill support).
private var captured: [String: Process] = [:] private var captured: [String: Process] = [:]
/// Hooks run by `kill` before terminating the child.
/// Used by `chartread` to park an XY head with `q\n`.
private var preKillHooks: [String: @Sendable () async -> Void] = [:]
/// Ids of currently-running children. /// Ids of currently-running children.
public var runningIDs: [String] { Array(children.keys) + captured.keys } public var runningIDs: [String] { Array(children.keys) + captured.keys }
@@ -105,6 +114,14 @@ public actor ProcessManager {
children[id] != nil || captured[id] != nil children[id] != nil || captured[id] != nil
} }
// MARK: - Pre-kill hooks
/// Register a hook to run before `kill(id:)` terminates the child.
/// The hook is removed once the child finalizes.
public func setPreKillHook(id: String, hook: @escaping @Sendable () async -> Void) {
preKillHooks[id] = hook
}
// MARK: - Spawn (streaming) // MARK: - Spawn (streaming)
/// Spawns a streaming child. Returns after spawn; callers wait for /// Spawns a streaming child. Returns after spawn; callers wait for
@@ -142,14 +159,6 @@ public actor ProcessManager {
stderrDecoder: ProcessLineDecoder() stderrDecoder: ProcessLineDecoder()
) )
do {
try process.run()
} catch {
children.removeValue(forKey: id)
emit(.error(id: id, message: error.localizedDescription))
throw ProcessError.spawnFailed("\(binary.path): \(error.localizedDescription)")
}
let stdoutHandle = stdoutPipe.fileHandleForReading let stdoutHandle = stdoutPipe.fileHandleForReading
let stderrHandle = stderrPipe.fileHandleForReading let stderrHandle = stderrPipe.fileHandleForReading
stdoutHandle.readabilityHandler = { [weak self] handle in stdoutHandle.readabilityHandler = { [weak self] handle in
@@ -167,6 +176,15 @@ public actor ProcessManager {
guard let self else { return } guard let self else { return }
Task { await self.didTerminate(id: id, code: proc.terminationStatus) } Task { await self.didTerminate(id: id, code: proc.terminationStatus) }
} }
do {
try process.run()
} catch {
preKillHooks.removeValue(forKey: id)
children.removeValue(forKey: id)
emit(.error(id: id, message: error.localizedDescription))
throw ProcessError.spawnFailed("\(binary.path): \(error.localizedDescription)")
}
} }
// MARK: - Spawn (captured) // MARK: - Spawn (captured)
@@ -198,41 +216,114 @@ public actor ProcessManager {
"spawn(captured) \(id): \(binary.path) \(LogSanitizer.sanitizeArgs(arguments))" "spawn(captured) \(id): \(binary.path) \(LogSanitizer.sanitizeArgs(arguments))"
) )
// Register before run() so a concurrent duplicate spawn fails. // Register and set up the termination hand-off before run() so
// a very fast exit is never missed (#50, #52).
captured[id] = process captured[id] = process
let capturedProcess = process
// Box is local and synchronised with an NSLock; the @unchecked
// Sendable annotation is safe because all access is under the lock.
final class Box: @unchecked Sendable {
private let lock = NSLock()
private var status: Int32?
private var continuation: CheckedContinuation<Int32, Never>?
/// Try to resume an already-stored continuation with the exit
/// status. Returns true if a continuation was resumed.
func resume(with status: Int32) -> Bool {
lock.lock()
if let cont = continuation {
continuation = nil
lock.unlock()
cont.resume(returning: status)
return true
}
self.status = status
lock.unlock()
return false
}
/// Store a continuation, returning any status that arrived
/// before it. The caller must resume with the returned status.
func store(_ continuation: CheckedContinuation<Int32, Never>) -> Int32? {
lock.lock()
if let status = status {
self.status = nil
self.continuation = nil
lock.unlock()
return status
}
self.continuation = continuation
// A fast exit may have raced past the first nil-check.
if let status = status {
self.status = nil
self.continuation = nil
lock.unlock()
return status
}
lock.unlock()
return nil
}
}
let box = Box()
capturedProcess.terminationHandler = { proc in
_ = box.resume(with: proc.terminationStatus)
}
do { do {
try process.run() try process.run()
} catch { } catch {
_ = box.resume(with: -1)
captured.removeValue(forKey: id) captured.removeValue(forKey: id)
preKillHooks.removeValue(forKey: id)
emit(.error(id: id, message: error.localizedDescription)) emit(.error(id: id, message: error.localizedDescription))
throw ProcessError.spawnFailed("\(binary.path): \(error.localizedDescription)") throw ProcessError.spawnFailed("\(binary.path): \(error.localizedDescription)")
} }
async let outData = Task.detached { // Close the parent write ends so readDataToEndOfFile() gets EOF
stdoutPipe.fileHandleForReading.readDataToEndOfFile() // as soon as the child exits; the child still has its own copies.
}.value try? stdoutPipe.fileHandleForWriting.close()
async let errData = Task.detached { try? stderrPipe.fileHandleForWriting.close()
stderrPipe.fileHandleForReading.readDataToEndOfFile()
}.value
let code = await withCheckedContinuation { continuation in return await withTaskCancellationHandler {
process.terminationHandler = { proc in async let outData = Task.detached {
continuation.resume(returning: proc.terminationStatus) stdoutPipe.fileHandleForReading.readDataToEndOfFile()
}.value
async let errData = Task.detached {
stderrPipe.fileHandleForReading.readDataToEndOfFile()
}.value
let code = await withCheckedContinuation { continuation in
if let status = box.store(continuation) {
continuation.resume(returning: status)
}
}
let (out, err) = await (outData, errData)
// Emit the real exit code once, regardless of whether kill()
// already removed the id from `captured`.
_ = captured.removeValue(forKey: id)
preKillHooks.removeValue(forKey: id)
emit(.exit(id: id, code: code))
return CapturedResult(
stdout: String(decoding: out, as: UTF8.self),
stderr: String(decoding: err, as: UTF8.self),
exitCode: code
)
} onCancel: { [weak self] in
// If the awaiting Task is cancelled, terminate the child so
// callers like runApplycal never replace a good profile with
// a truncated tmp.
if capturedProcess.isRunning {
capturedProcess.terminate()
}
Task { [weak self] in
await self?.kill(id: id)
} }
} }
let (out, err) = await (outData, errData)
// If kill() already reaped this child, its exit event went out.
if captured.removeValue(forKey: id) != nil {
emit(.exit(id: id, code: code))
}
return CapturedResult(
stdout: String(decoding: out, as: UTF8.self),
stderr: String(decoding: err, as: UTF8.self),
exitCode: code
)
} }
// MARK: - stdin // MARK: - stdin
@@ -255,39 +346,89 @@ public actor ProcessManager {
try sendStdin(id: id, bytes: Data(text.utf8)) try sendStdin(id: id, bytes: Data(text.utf8))
} }
// MARK: - Partial-line flush
/// Emits the current unterminated tail of a streaming child's stdout
/// and stderr as ordinary lines. Callers (e.g. `colprof`) use this
/// to flush progress dots without waiting for a newline.
public func flushPartialLine(id: String) {
guard var child = children[id], !child.finalized else { return }
if let tail = child.stdoutDecoder.flushPartial() {
if tail.hasPrefix(Self.rowColorsPrefix) {
let payload = Data(tail.dropFirst(Self.rowColorsPrefix.count).utf8)
emit(.jsonRow(id: id, payload: payload))
} else {
emit(.stdout(id: id, line: tail))
}
}
if let tail = child.stderrDecoder.flushPartial() {
emit(.stderr(id: id, line: tail))
}
children[id] = child
}
// MARK: - Kill // MARK: - Kill
/// Terminates a child. The `exit` event still fires exactly once. /// Terminates a child. First runs any registered pre-kill hook, then
/// stdin is dropped immediately so writers fail fast (docs/03 rule 7). /// drops stdin and signals the process. For streaming children the
public func kill(id: String) { /// `exit` event is emitted once both stdout and stderr EOFs have been
/// seen (or the watchdog finalizes). For captured children the real
/// exit code is emitted by `runCaptured` itself.
public func kill(id: String) async {
if let hook = preKillHooks.removeValue(forKey: id) {
await hook()
}
if var child = children[id] { if var child = children[id] {
try? child.stdin?.close() try? child.stdin?.close()
child.stdin = nil child.stdin = nil
children[id] = child children[id] = child
if child.process.isRunning { if child.process.isRunning {
child.process.terminate() child.process.terminate()
} else { } else if child.pendingExitCode == nil {
Task { await self.didTerminate(id: id, code: child.process.terminationStatus) } // The process already exited but `didTerminate` has not
// run; synthesize it so `maybeFinalize` can fire.
didTerminate(id: id, code: child.process.terminationStatus)
} }
return return
} }
if let process = captured[id] { if let process = captured[id] {
if process.isRunning { process.terminate() } if process.isRunning { process.terminate() }
if captured.removeValue(forKey: id) != nil { // Do not emit `.exit` here; `runCaptured` emits the real code
emit(.exit(id: id, code: process.terminationStatus)) // after the process reaps.
} return
} }
} }
/// Terminates every running child; returns how many were signaled /// Terminates every running child; returns how many were signaled
/// (`kill_all_processes`, docs/03). Mandatory on app exit (#147/#149). /// (`kill_all_processes`, docs/03). Mandatory on app exit (#147/#149).
@discardableResult @discardableResult
public func killAll() -> Int { public func killAll() async -> Int {
let ids = Array(children.keys) + Array(captured.keys) let ids = runningIDs
for id in ids { kill(id: id) } for id in ids { await kill(id: id) }
return ids.count return ids.count
} }
// MARK: - Force kill (SIGKILL fallback)
/// Sends `SIGKILL` to a streaming child if it is still running.
/// Used by the finalization watchdog when a graceful `terminate()`
/// does not cause the process to exit.
public func forceKill(id: String) {
guard let child = children[id],
!child.finalized,
child.process.isRunning
else { return }
let pid = child.process.processIdentifier
guard pid > 0 else { return }
_ = Darwin.kill(pid, SIGKILL)
}
// MARK: - Internals // MARK: - Internals
private func childEnvironment(extra: [String: String]) -> [String: String] { private func childEnvironment(extra: [String: String]) -> [String: String] {
@@ -339,6 +480,16 @@ public actor ProcessManager {
child.pendingExitCode = code child.pendingExitCode = code
try? child.stdin?.close() try? child.stdin?.close()
child.stdin = nil child.stdin = nil
// Start a watchdog in case the `readabilityHandler` EOFs never
// arrive after the process exits (e.g. a hung pipe).
child.finalizeTask = Task { [weak self] in
try? await Task.sleep(for: .seconds(2))
guard let self else { return }
await self.forceKill(id: id)
await self.forceFinalize(id: id)
}
children[id] = child children[id] = child
maybeFinalize(id: id) maybeFinalize(id: id)
} }
@@ -351,8 +502,12 @@ public actor ProcessManager {
child.stdoutEOF, child.stderrEOF, child.stdoutEOF, child.stderrEOF,
!child.finalized !child.finalized
else { return } else { return }
child.finalized = true child.finalized = true
child.finalizeTask?.cancel()
child.finalizeTask = nil
children.removeValue(forKey: id) children.removeValue(forKey: id)
preKillHooks.removeValue(forKey: id)
// Flush unterminated tail lines. // Flush unterminated tail lines.
if var decoder = Optional(child.stdoutDecoder), if var decoder = Optional(child.stdoutDecoder),
@@ -369,4 +524,20 @@ public actor ProcessManager {
} }
emit(.exit(id: id, code: code)) emit(.exit(id: id, code: code))
} }
/// Forces finalization even when one or both EOFs are missing.
/// Used by the `didTerminate` watchdog.
private func forceFinalize(id: String) {
guard var child = children[id], !child.finalized else { return }
if child.pendingExitCode == nil {
child.pendingExitCode = -9
}
child.stdoutEOF = true
child.stderrEOF = true
child.finalizeTask?.cancel()
child.finalizeTask = nil
children[id] = child
maybeFinalize(id: id)
}
} }
@@ -2,32 +2,40 @@ import Foundation
/// Computes a consecutive-breach warning from verification history. /// Computes a consecutive-breach warning from verification history.
/// ///
/// A drift alert triggers when there are at least two `poor` records on /// A drift alert triggers when the most recent chronologically consecutive
/// distinct calendar days, or two `poor` records at least one hour apart. /// poor records form a run of at least two, and the first and last of that
/// run are on distinct UTC days or at least one hour apart.
public enum DriftAlert { public enum DriftAlert {
/// Returns an alert message, or `nil` when no consecutive breach exists. /// Returns an alert message, or `nil` when no consecutive breach exists.
public static func compute(from records: [VerificationRecord]) -> String? { public static func compute(from records: [VerificationRecord]) -> String? {
let poor = records // Work in chronological order.
.filter { $0.status == .poor } let chronological = records.sorted { $0.timestamp < $1.timestamp }
.sorted { $0.timestamp < $1.timestamp }
guard poor.count >= 2 else { return nil } // Build the longest suffix of consecutive `.poor` records.
// Non-poor records break the run, so we stop at the first non-poor
for i in 0..<poor.count { // encountered from the end.
for j in (i + 1)..<poor.count { var run: [VerificationRecord] = []
let a = poor[i] for record in chronological.reversed() {
let b = poor[j] if record.status == .poor {
run.insert(record, at: 0)
let sameDay = Calendar.utc.isDate(a.timestamp, inSameDayAs: b.timestamp) } else {
let oneHour = b.timestamp.timeIntervalSince(a.timestamp) >= 3600 break
if !sameDay || oneHour {
return "Drift alert: poor results between \(a.id) and \(b.id)."
}
} }
} }
guard run.count >= 2 else { return nil }
let first = run.first!
let last = run.last!
let sameDay = Calendar.utc.isDate(first.timestamp, inSameDayAs: last.timestamp)
let oneHour = last.timestamp.timeIntervalSince(first.timestamp) >= 3600
if !sameDay || oneHour {
return "Drift alert: poor results between \(first.id) and \(last.id)."
}
return nil return nil
} }
} }
@@ -33,10 +33,32 @@ public enum ProfileInstallError: LocalizedError, Equatable, Sendable {
/// Installs an ICC/ICM profile into the OS colour store. /// Installs an ICC/ICM profile into the OS colour store.
public enum ProfileInstaller { public enum ProfileInstaller {
/// Resolves the destination URL that `install` would write to for the
/// given source and options, without copying anything. Useful for
/// collision previews in the UI.
public static func resolveDestinationURL(
for config: InstallProfileConfig,
fileManager: FileManager = .default
) throws -> URL {
let sourceURL = config.sourceURL
let ext = sourceURL.pathExtension.lowercased()
guard ext == "icc" || ext == "icm" else {
throw ProfileInstallError.sourceNotProfile
}
try validateSourceURL(sourceURL)
let destDir = destinationDirectory(for: config.options, fileManager: fileManager)
return destDir.appendingPathComponent(sourceURL.lastPathComponent)
}
/// Installs `sourceURL` into `~/Library/ColorSync/Profiles` or /// Installs `sourceURL` into `~/Library/ColorSync/Profiles` or
/// `/Library/ColorSync/Profiles`. Always copies, never moves. /// `/Library/ColorSync/Profiles`. Always copies, never moves.
public static func install(config: InstallProfileConfig) throws -> InstallProfileResult { public static func install(
let fm = FileManager.default config: InstallProfileConfig,
fileManager: FileManager = .default
) throws -> InstallProfileResult {
let fm = fileManager
// Source validation. // Source validation.
let sourceURL = config.sourceURL let sourceURL = config.sourceURL
@@ -55,38 +77,37 @@ public enum ProfileInstaller {
throw ProfileInstallError.sourceTooSmall throw ProfileInstallError.sourceTooSmall
} }
// Stem security. try validateSourceURL(sourceURL)
let stem = sourceURL.deletingPathExtension().lastPathComponent
guard !stem.contains("..") && !stem.contains("/") && !stem.contains("\\") else {
throw ProfileInstallError.unsafeStem(stem)
}
// Destination directory. // Destination directory.
let destDir: URL let destURL = try resolveDestinationURL(for: config, fileManager: fm)
if config.options.preferSystem { try? fm.createDirectory(
destDir = URL(fileURLWithPath: "/Library/ColorSync/Profiles") at: destURL.deletingLastPathComponent(),
} else { withIntermediateDirectories: true
let home = fm.homeDirectoryForCurrentUser )
destDir = home.appendingPathComponent("Library/ColorSync/Profiles")
}
// Ensure parent exists.
try? fm.createDirectory(at: destDir, withIntermediateDirectories: true)
let destURL = destDir.appendingPathComponent("\(stem).icc")
// Collision resolution. // Collision resolution.
let destExists = fm.fileExists(atPath: destURL.path) let destExists = fm.fileExists(atPath: destURL.path)
if destExists { if destExists {
if config.options.forceOverwrite { if config.options.forceOverwrite {
// Continue to overwrite path. return try performInstall(
from: sourceURL,
to: destURL,
options: config.options,
fileManager: fm,
overwritten: true,
renamed: false
)
} else if config.options.collisionPolicy == .rename { } else if config.options.collisionPolicy == .rename {
let epoch = Int(Date().timeIntervalSince1970) let epoch = Int(Date().timeIntervalSince1970)
let renamedURL = destDir.appendingPathComponent("\(stem)-\(epoch).icc") let stem = sourceURL.deletingPathExtension().lastPathComponent
let renamedURL = destURL.deletingLastPathComponent()
.appendingPathComponent("\(stem)-\(epoch).\(ext)")
return try performInstall( return try performInstall(
from: sourceURL, from: sourceURL,
to: renamedURL, to: renamedURL,
options: config.options, options: config.options,
fileManager: fm,
overwritten: false, overwritten: false,
renamed: true renamed: true
) )
@@ -102,19 +123,53 @@ public enum ProfileInstaller {
from: sourceURL, from: sourceURL,
to: destURL, to: destURL,
options: config.options, options: config.options,
overwritten: destExists, fileManager: fm,
overwritten: false,
renamed: false renamed: false
) )
} }
// MARK: - Private helpers
private static func validateSourceURL(_ sourceURL: URL) throws {
let path = sourceURL.path
let stem = sourceURL.deletingPathExtension().lastPathComponent
// Reject backslashes anywhere in the path.
guard !path.contains("\\") else {
throw ProfileInstallError.unsafeStem(stem)
}
// Reject any path component that is literally "." or "..".
// This allows names like "foo..bar" while blocking real traversal.
for component in sourceURL.pathComponents {
if component == "." || component == ".." {
throw ProfileInstallError.unsafeStem(stem)
}
}
}
private static func destinationDirectory(
for options: InstallProfileOptions,
fileManager: FileManager
) -> URL {
if options.preferSystem {
return URL(fileURLWithPath: "/Library/ColorSync/Profiles")
} else {
return fileManager.homeDirectoryForCurrentUser
.appendingPathComponent("Library/ColorSync/Profiles")
}
}
private static func performInstall( private static func performInstall(
from sourceURL: URL, from sourceURL: URL,
to destURL: URL, to destURL: URL,
options: InstallProfileOptions, options: InstallProfileOptions,
fileManager: FileManager,
overwritten: Bool, overwritten: Bool,
renamed: Bool renamed: Bool
) throws -> InstallProfileResult { ) throws -> InstallProfileResult {
let fm = FileManager.default let fm = fileManager
let tmpURL = destURL.appendingPathExtension("iccery-install.tmp") let tmpURL = destURL.appendingPathExtension("iccery-install.tmp")
// Remove stale tmp. // Remove stale tmp.
@@ -123,6 +178,13 @@ public enum ProfileInstaller {
do { do {
try fm.copyItem(at: sourceURL, to: tmpURL) try fm.copyItem(at: sourceURL, to: tmpURL)
let attrs = try? fm.attributesOfItem(atPath: tmpURL.path)
let tmpSize = attrs?[.size] as? UInt64 ?? 0
guard tmpSize >= 128 else {
try? fm.removeItem(at: tmpURL)
throw ProfileInstallError.sourceTooSmall
}
if fm.fileExists(atPath: destURL.path) { if fm.fileExists(atPath: destURL.path) {
_ = try fm.replaceItemAt(destURL, withItemAt: tmpURL) _ = try fm.replaceItemAt(destURL, withItemAt: tmpURL)
} else { } else {
@@ -135,6 +197,10 @@ public enum ProfileInstaller {
if destURL.path.hasPrefix("/Library/") && !fm.fileExists(atPath: destURL.path) { if destURL.path.hasPrefix("/Library/") && !fm.fileExists(atPath: destURL.path) {
throw ProfileInstallError.systemRequiresAdminRights throw ProfileInstallError.systemRequiresAdminRights
} }
if let installError = error as? ProfileInstallError {
throw installError
}
throw ProfileInstallError.copyFailed(error.localizedDescription) throw ProfileInstallError.copyFailed(error.localizedDescription)
} }
@@ -57,10 +57,12 @@ public actor VerificationHistoryStore {
/// Appends a record, trims to capacity, and writes atomically. /// Appends a record, trims to capacity, and writes atomically.
/// ///
/// Returns the trimmed list, or `nil` if a write error occurs so the /// Loads the existing history first and propagates any load error so an
/// caller can surface the failure without replacing the in-memory list. /// unparseable file is never overwritten.
@discardableResult @discardableResult
public func append(_ record: VerificationRecord) throws -> [VerificationRecord] { public func append(_ record: VerificationRecord) throws -> [VerificationRecord] {
try load()
var updated = records var updated = records
updated.append(record) updated.append(record)
if updated.count > capacity { if updated.count > capacity {
@@ -305,6 +305,10 @@ final class MeasurementWorkflowViewModel {
environment.runner.cancelChartread(basename: basename, isXY: selectedInstrument.isXY) environment.runner.cancelChartread(basename: basename, isXY: selectedInstrument.isXY)
chartreadTask?.cancel() chartreadTask?.cancel()
isChartreadRunning = false isChartreadRunning = false
chartreadState = .idle
currentPrompt = nil
requestedWarningKey = nil
showRemoveSheetNotice = false
} }
func sendWarningKey(_ key: String) { func sendWarningKey(_ key: String) {
+43 -24
View File
@@ -81,6 +81,15 @@ final class ProfileWorkflowViewModel {
init(wizard: WizardViewModel, environment: AppEnvironment) { init(wizard: WizardViewModel, environment: AppEnvironment) {
self.wizard = wizard self.wizard = wizard
self.environment = environment self.environment = environment
restoreCreatedProfileURL()
}
/// Restores `createdProfileURL` from the wizard artefacts or by probing
/// the working directory for an existing `.icc`/`.icm` (#52).
func restoreCreatedProfileURL() {
let cwd = wizard.effectiveWorkingDirectory ?? PathSecurity.resolveSafeCwd(nil)
createdProfileURL = wizard.artefacts.profilePath
?? ArtefactProbe.resolveProfile(basename: wizard.basename, cwd: cwd)
} }
// MARK: - Derived // MARK: - Derived
@@ -206,6 +215,7 @@ final class ProfileWorkflowViewModel {
calibrationPath: self.calibrationFile, calibrationPath: self.calibrationFile,
inputProfileURL: url inputProfileURL: url
) )
assert(!applyConfig.unapply, "applycal unapply is not supported in v2.0")
finalProfileURL = try await runner.runApplycal(config: applyConfig) finalProfileURL = try await runner.runApplycal(config: applyConfig)
self.colprofLog.append("Calibration embedded: \(self.calibrationFile)") self.colprofLog.append("Calibration embedded: \(self.calibrationFile)")
} }
@@ -256,7 +266,15 @@ final class ProfileWorkflowViewModel {
// MARK: - Stage 5: verify profile // MARK: - Stage 5: verify profile
var knownPrinters: [String] { var knownPrinters: [String] {
Array(Set(verificationHistory.map { $0.printerName })).sorted() var names = Set<String>()
for record in verificationHistory {
if record.printerName.isEmpty {
names.insert("Unknown")
} else {
names.insert(record.printerName)
}
}
return Array(names).sorted()
} }
func loadHistory() { func loadHistory() {
@@ -333,10 +351,11 @@ final class ProfileWorkflowViewModel {
let timestamp = Date() let timestamp = Date()
let id = "vr-\(Int(timestamp.timeIntervalSince1970))-\(Self.nextSeq())" let id = "vr-\(Int(timestamp.timeIntervalSince1970))-\(Self.nextSeq())"
let printerName = wizard.printerName?.isEmpty == false ? wizard.printerName! : "Unknown"
return VerificationRecord( return VerificationRecord(
id: id, id: id,
profileName: createdProfileURL?.lastPathComponent ?? wizard.basename, profileName: createdProfileURL?.lastPathComponent ?? wizard.basename,
printerName: wizard.printerName ?? "", printerName: printerName,
avgDE: avg, avgDE: avg,
maxDE: max, maxDE: max,
rmsDE: rms, rmsDE: rms,
@@ -381,24 +400,32 @@ final class ProfileWorkflowViewModel {
openColorPanel: settings.openColorPanelAfterInstall openColorPanel: settings.openColorPanelAfterInstall
) )
let destURL = installDestination(for: sourceURL, options: options) do {
let collision = FileManager.default.fileExists(atPath: destURL.path) let config = InstallProfileConfig(sourceURL: sourceURL, options: options)
let destURL = try ProfileInstaller.resolveDestinationURL(for: config)
let collision = FileManager.default.fileExists(atPath: destURL.path)
if collision && settings.askBeforeOverwriteProfile { if collision && settings.askBeforeOverwriteProfile {
pendingInstallOptions = options pendingInstallOptions = options
installCollisionMessage = "A profile named \(destURL.lastPathComponent) already exists." installCollisionMessage = "A profile named \(destURL.lastPathComponent) already exists."
showingInstallCollision = true showingInstallCollision = true
return return
}
runInstall(sourceURL: sourceURL, options: options)
} catch {
wizard.showNotice(
"Install failed: \(error.localizedDescription)",
kind: .error
)
} }
runInstall(sourceURL: sourceURL, options: options)
} }
func resolveInstallCollision(policy: ProfileCollisionPolicy) { func resolveInstallCollision(policy: ProfileCollisionPolicy) {
showingInstallCollision = false showingInstallCollision = false
guard let sourceURL = createdProfileURL, guard let sourceURL = createdProfileURL,
var options = pendingInstallOptions else { return } var options = pendingInstallOptions else { return }
options.collisionPolicy = policy
if policy == .cancel { if policy == .cancel {
installResult = InstallProfileResult( installResult = InstallProfileResult(
destPath: "", destPath: "",
@@ -410,20 +437,12 @@ final class ProfileWorkflowViewModel {
) )
return return
} }
runInstall(sourceURL: sourceURL, options: options)
}
private func installDestination(for sourceURL: URL, options: InstallProfileOptions) -> URL { options.collisionPolicy = policy
let stem = sourceURL.deletingPathExtension().lastPathComponent if policy == .overwrite {
let fm = FileManager.default options.forceOverwrite = true
let destDir: URL
if options.preferSystem {
destDir = URL(fileURLWithPath: "/Library/ColorSync/Profiles")
} else {
destDir = fm.homeDirectoryForCurrentUser
.appendingPathComponent("Library/ColorSync/Profiles")
} }
return destDir.appendingPathComponent("\(stem).icc") runInstall(sourceURL: sourceURL, options: options)
} }
private func runInstall(sourceURL: URL, options: InstallProfileOptions) { private func runInstall(sourceURL: URL, options: InstallProfileOptions) {
+5 -12
View File
@@ -210,7 +210,9 @@ struct Stage3View: View {
.accessibilityIdentifier("btnCalibrate") .accessibilityIdentifier("btnCalibrate")
case .awaitingStrip: case .awaitingStrip:
Button("Trigger") { model.calibrate() } Button("Trigger") { model.calibrate() }
.accessibilityIdentifier("btnCalibrate") .accessibilityIdentifier("btnTrigger")
Button("Done & Save") { model.doneAndSave() }
.accessibilityIdentifier("btnDoneReadEarly")
case .tablePlaceSheet, .tableAlign, .promptContinue, .warning: case .tablePlaceSheet, .tableAlign, .promptContinue, .warning:
Button(continueTitle) { model.accept() } Button(continueTitle) { model.accept() }
.accessibilityIdentifier("btnAccept") .accessibilityIdentifier("btnAccept")
@@ -224,16 +226,6 @@ struct Stage3View: View {
EmptyView() EmptyView()
} }
if model.chartreadState == .awaitingStrip || model.chartreadState == .allStripsRead {
Button("Done & Save") { model.doneAndSave() }
.accessibilityIdentifier("btnDoneRead")
}
if model.chartreadState == .error {
Button("Retry") { model.retry() }
.accessibilityIdentifier("btnRetry")
}
Button("Cancel") { model.cancelRead() } Button("Cancel") { model.cancelRead() }
.accessibilityIdentifier("btnCancel") .accessibilityIdentifier("btnCancel")
} }
@@ -374,7 +366,7 @@ struct Stage3View: View {
Button("Finish & Average") { Button("Finish & Average") {
model.finishAndAverage() model.finishAndAverage()
} }
.disabled(!model.isFinished || model.isFinishing) .disabled(!model.canFinish || model.isFinishing)
.accessibilityIdentifier("btnFinishAndAverage") .accessibilityIdentifier("btnFinishAndAverage")
} }
@@ -386,6 +378,7 @@ struct Stage3View: View {
} }
.padding(16) .padding(16)
.background(Theme.panel) .background(Theme.panel)
.accessibilityElement(children: .contain)
.accessibilityIdentifier("chartreadAveragingPanel") .accessibilityIdentifier("chartreadAveragingPanel")
} }
} }
+1
View File
@@ -18,6 +18,7 @@ struct Stage4View: View {
} }
.frame(maxWidth: .infinity, maxHeight: .infinity) .frame(maxWidth: .infinity, maxHeight: .infinity)
.background(Theme.background) .background(Theme.background)
.onAppear { model.restoreCreatedProfileURL() }
} }
// MARK: - Header // MARK: - Header
+5 -2
View File
@@ -21,7 +21,10 @@ struct Stage5View: View {
} }
.frame(maxWidth: .infinity, maxHeight: .infinity) .frame(maxWidth: .infinity, maxHeight: .infinity)
.background(Theme.background) .background(Theme.background)
.onAppear { model.loadHistory() } .onAppear {
model.restoreCreatedProfileURL()
model.loadHistory()
}
.alert("Install profile", isPresented: $model.showingInstallCollision) { .alert("Install profile", isPresented: $model.showingInstallCollision) {
Button("Overwrite", role: .destructive) { Button("Overwrite", role: .destructive) {
model.resolveInstallCollision(policy: .overwrite) model.resolveInstallCollision(policy: .overwrite)
@@ -70,7 +73,7 @@ struct Stage5View: View {
.accessibilityIdentifier("driftAlert") .accessibilityIdentifier("driftAlert")
} }
if let warning = model.profcheckWarning, !warning.isEmpty, model.driftAlert == nil { if let warning = model.profcheckWarning, !warning.isEmpty {
Text("\(warning)") Text("\(warning)")
.font(.caption) .font(.caption)
.padding(.horizontal, 8) .padding(.horizontal, 8)
@@ -15,16 +15,16 @@ struct ApplycalArgsTests {
#expect(args == ["-v", "-a", "/tmp/cal.cal", "/tmp/profile.icc"]) #expect(args == ["-v", "-a", "/tmp/cal.cal", "/tmp/profile.icc"])
} }
@Test("Unapply is never sent from build") @Test("Unapply is emitted when the caller explicitly sets it")
func unapplyNotEmitted() throws { func unapplyEmittedWhenConfigSet() throws {
let config = ApplycalConfig( let config = ApplycalConfig(
calibrationPath: "/tmp/cal.cal", calibrationPath: "/tmp/cal.cal",
inputProfileURL: URL(fileURLWithPath: "/tmp/profile.icc"), inputProfileURL: URL(fileURLWithPath: "/tmp/profile.icc"),
unapply: true unapply: true
) )
let args = try ApplycalArgs.build(config: config) let args = try ApplycalArgs.build(config: config)
// Builder intentionally emits -u because config can set it, but // Builder emits -u only when the caller explicitly sets unapply.
// the UI layer never passes unapply: true in v2.0. // The UI layer never passes unapply: true in v2.0.
#expect(args == ["-v", "-u", "/tmp/cal.cal", "/tmp/profile.icc"]) #expect(args == ["-v", "-u", "/tmp/cal.cal", "/tmp/profile.icc"])
} }
} }
+43 -1
View File
@@ -38,7 +38,7 @@ struct DriftAlertTests {
#expect(DriftAlert.compute(from: [day1, day2]) != nil) #expect(DriftAlert.compute(from: [day1, day2]) != nil)
} }
@Test("Non-poor results do not trigger") @Test("Non-poor records do not trigger")
func nonPoor() { func nonPoor() {
let records = [ let records = [
record(avg: 1.0, at: 0), record(avg: 1.0, at: 0),
@@ -47,6 +47,48 @@ struct DriftAlertTests {
#expect(DriftAlert.compute(from: records) == nil) #expect(DriftAlert.compute(from: records) == nil)
} }
@Test("Non-poor records break the consecutive poor run")
func nonPoorBreaksRun() {
let records = [
record(avg: 4.0, at: 0), // poor
record(avg: 4.5, at: 86400), // poor, far apart
record(avg: 1.0, at: 90000), // good breaks the run
record(avg: 4.0, at: 92000) // poor, recent but close to previous poor
]
#expect(DriftAlert.compute(from: records) == nil)
}
@Test("Only the final consecutive poor run is considered")
func onlySuffixRun() {
let records = [
record(avg: 4.0, at: 0), // poor
record(avg: 4.5, at: 18000), // poor, > 1h from first
record(avg: 1.0, at: 20000), // good breaks the run
record(avg: 4.0, at: 25000), // poor
record(avg: 4.5, at: 26000) // poor, < 1h and same day
]
#expect(DriftAlert.compute(from: records) == nil)
}
@Test("Final consecutive poor run alerts when far apart")
func suffixRunAlerts() {
let records = [
record(avg: 1.0, at: 0), // good
record(avg: 4.0, at: 1000), // poor
record(avg: 4.5, at: 4600) // poor, 1h after previous
]
#expect(DriftAlert.compute(from: records) != nil)
}
@Test("A single final poor record after good records does not alert")
func singleFinalPoor() {
let records = [
record(avg: 1.0, at: 0),
record(avg: 4.0, at: 86400)
]
#expect(DriftAlert.compute(from: records) == nil)
}
private func record(avg: Double, at offset: TimeInterval) -> VerificationRecord { private func record(avg: Double, at offset: TimeInterval) -> VerificationRecord {
VerificationRecord( VerificationRecord(
id: "vr-\(Int(offset))", id: "vr-\(Int(offset))",
+140 -44
View File
@@ -2,42 +2,165 @@ import Foundation
import Testing import Testing
@testable import ICCeryCore @testable import ICCeryCore
/// A `FileManager` subclass that reports a temporary directory as the
/// user home, so `ProfileInstaller` can be tested without writing to the
/// real `~/Library/ColorSync/Profiles`.
private final class TestFileManager: FileManager {
let tempHome: URL
init(home: URL) {
self.tempHome = home
super.init()
}
override var homeDirectoryForCurrentUser: URL {
tempHome
}
}
@Suite("ProfileInstaller") @Suite("ProfileInstaller")
struct ProfileInstallerTests { struct ProfileInstallerTests {
@Test("Copies .icc to user ColorSync folder") private func makeTempDir() throws -> URL {
func userInstall() throws {
let fm = FileManager.default let fm = FileManager.default
let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString)
try fm.createDirectory(at: tmp, withIntermediateDirectories: true) try fm.createDirectory(at: tmp, withIntermediateDirectories: true)
return tmp
}
let source = tmp.appendingPathComponent("test.icc") private func makeSource(
let iccData = Data(repeating: 0, count: 256) at dir: URL,
try iccData.write(to: source) name: String,
bytes: [UInt8] = Array(repeating: 0, count: 256)
) throws -> URL {
let url = dir.appendingPathComponent(name)
let data = Data(bytes)
try data.write(to: url)
return url
}
let colorsync = tmp.appendingPathComponent("Library/ColorSync/Profiles") @Test("Installs .icc to user ColorSync folder")
func userInstall() throws {
let fm = FileManager.default
let tmp = try makeTempDir()
let testFM = TestFileManager(home: tmp)
let source = try makeSource(at: tmp, name: "test.icc")
// Inject a user profile install by replacing the home directory let result = try ProfileInstaller.install(
// is not practical; instead exercise Core validation on a config: InstallProfileConfig(sourceURL: source),
// temp-only path via the file URL safety checks and the public fileManager: testFM
// install against a writable system-like path is tested below. )
#expect(result.registered)
#expect(!result.overwritten)
#expect(!result.renamed)
#expect(result.destPath.hasSuffix("test.icc"))
#expect(fm.fileExists(atPath: result.destPath))
}
@Test("Overwrite succeeds and replaces the existing file")
func overwriteSucceeds() throws {
let fm = FileManager.default
let tmp = try makeTempDir()
let testFM = TestFileManager(home: tmp)
let source = try makeSource(at: tmp, name: "m5_profile.icc", bytes: (0..<256).map { UInt8($0) })
// First install.
let first = try ProfileInstaller.install(
config: InstallProfileConfig(sourceURL: source),
fileManager: testFM
)
#expect(!first.overwritten)
// Change the source contents.
let newBytes: [UInt8] = (0..<256).map { UInt8(($0 + 100) % 256) }
try Data(newBytes).write(to: source)
let options = InstallProfileOptions(
forceOverwrite: true,
preferSystem: false,
collisionPolicy: .overwrite,
openColorPanel: false
)
let second = try ProfileInstaller.install(
config: InstallProfileConfig(sourceURL: source, options: options),
fileManager: testFM
)
#expect(second.overwritten)
#expect(!second.renamed)
#expect(fm.fileExists(atPath: second.destPath))
let installed = try Data(contentsOf: URL(fileURLWithPath: second.destPath))
#expect(Array(installed) == newBytes)
}
@Test("Preserves .icm source extension")
func preservesIcmExtension() throws {
let tmp = try makeTempDir()
let testFM = TestFileManager(home: tmp)
let source = try makeSource(at: tmp, name: "m5_profile.icm")
let result = try ProfileInstaller.install(
config: InstallProfileConfig(sourceURL: source),
fileManager: testFM
)
#expect(URL(fileURLWithPath: result.destPath).pathExtension == "icm")
#expect(result.destPath.hasSuffix("m5_profile.icm"))
}
@Test("Rejects parent traversal in source path")
func rejectsParentTraversal() throws {
let fm = FileManager.default
let tmp = try makeTempDir()
// Create a real file in the parent of `tmp` with a path that contains
// a literal ".." component.
let parent = tmp.deletingLastPathComponent()
let naughtyName = "naughty-\(UUID().uuidString).icc"
let realFile = parent.appendingPathComponent(naughtyName)
_ = try makeSource(at: parent, name: naughtyName)
defer { try? fm.removeItem(at: realFile) }
let sourceURL = tmp
.appendingPathComponent("..")
.appendingPathComponent(naughtyName)
#expect(fm.fileExists(atPath: sourceURL.path))
// For this unit test, validate the stem security and source rules.
let unsafe = tmp.appendingPathComponent("bad..stem.icc")
try Data(repeating: 0, count: 256).write(to: unsafe)
do { do {
_ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: unsafe)) _ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: sourceURL))
Issue.record("Expected unsafeStem error") Issue.record("Expected unsafeStem error")
} catch let error as ProfileInstallError { } catch let error as ProfileInstallError {
if case .unsafeStem = error { } else { Issue.record("Expected unsafeStem, got \(error)") } if case .unsafeStem = error { } else { Issue.record("Expected unsafeStem, got \(error)") }
} catch { } catch {
Issue.record("Unexpected error type: \(error)") Issue.record("Unexpected error type: \(error)")
} }
}
@Test("Allows stems with consecutive dots like foo..bar")
func allowsDoubleDotStem() throws {
let fm = FileManager.default
let tmp = try makeTempDir()
let testFM = TestFileManager(home: tmp)
let source = try makeSource(at: tmp, name: "foo..bar.icc")
let result = try ProfileInstaller.install(
config: InstallProfileConfig(sourceURL: source),
fileManager: testFM
)
#expect(result.destPath.hasSuffix("foo..bar.icc"))
#expect(fm.fileExists(atPath: result.destPath))
}
@Test("Rejects source files that are too small")
func rejectsSmallSource() throws {
let tmp = try makeTempDir()
let source = tmp.appendingPathComponent("tiny.icc")
try Data(repeating: 0, count: 64).write(to: source)
let small = tmp.appendingPathComponent("tiny.icc")
try Data(repeating: 0, count: 64).write(to: small)
do { do {
_ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: small)) _ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: source))
Issue.record("Expected sourceTooSmall error") Issue.record("Expected sourceTooSmall error")
} catch let error as ProfileInstallError { } catch let error as ProfileInstallError {
if case .sourceTooSmall = error { } else { Issue.record("Expected sourceTooSmall, got \(error)") } if case .sourceTooSmall = error { } else { Issue.record("Expected sourceTooSmall, got \(error)") }
@@ -45,31 +168,4 @@ struct ProfileInstallerTests {
Issue.record("Unexpected error type: \(error)") Issue.record("Unexpected error type: \(error)")
} }
} }
@Test("Installs into a temp user folder and preserves source")
func tempInstallPreservesSource() throws {
let fm = FileManager.default
let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString)
try fm.createDirectory(at: tmp, withIntermediateDirectories: true)
let source = tmp.appendingPathComponent("m5_profile.icc")
try Data(repeating: 0, count: 256).write(to: source)
let destDir = tmp.appendingPathComponent("ColorSync/Profiles")
try fm.createDirectory(at: destDir, withIntermediateDirectories: true)
// There is no public API to override the home directory, so
// test the copy mechanism directly via file operations.
let dest = destDir.appendingPathComponent("m5_profile.icc")
let tmpDest = dest.appendingPathExtension("iccery-install.tmp")
try fm.copyItem(at: source, to: tmpDest)
if fm.fileExists(atPath: dest.path) {
_ = try fm.replaceItemAt(dest, withItemAt: tmpDest)
} else {
try fm.moveItem(at: tmpDest, to: dest)
}
#expect(fm.fileExists(atPath: source.path))
#expect(fm.fileExists(atPath: dest.path))
}
} }
@@ -51,6 +51,86 @@ struct VerificationHistoryStoreTests {
} }
} }
@Test("Append loads existing records first")
func appendLoadsExisting() async throws {
let fm = FileManager.default
let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString)
try fm.createDirectory(at: tmp, withIntermediateDirectories: true)
let url = tmp.appendingPathComponent("verification_history.json")
// Pre-populate the store on disk.
let existing = VerificationRecord(
id: "vr-existing",
profileName: "p",
printerName: "",
avgDE: 1.0,
maxDE: 1.0,
rmsDE: 1.0,
patchCount: 1,
status: .good,
timestamp: Date(timeIntervalSince1970: 0)
)
let store1 = VerificationHistoryStore(url: url)
_ = try await store1.append(existing)
// A fresh store appending a new record must keep the existing one.
let store2 = VerificationHistoryStore(url: url)
let new = VerificationRecord(
id: "vr-new",
profileName: "p",
printerName: "",
avgDE: 2.0,
maxDE: 2.0,
rmsDE: 2.0,
patchCount: 2,
status: .good,
timestamp: Date(timeIntervalSince1970: 10)
)
_ = try await store2.append(new)
let all = await store2.all()
#expect(all.count == 2)
#expect(all.contains { $0.id == "vr-existing" })
#expect(all.contains { $0.id == "vr-new" })
}
@Test("Append does not overwrite an unparseable file")
func appendPreservesUnparseableFile() async {
let fm = FileManager.default
let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString)
try? fm.createDirectory(at: tmp, withIntermediateDirectories: true)
let url = tmp.appendingPathComponent("verification_history.json")
let badJSON = "not json"
try? badJSON.write(to: url, atomically: true, encoding: .utf8)
let store = VerificationHistoryStore(url: url)
let record = VerificationRecord(
id: "vr-new",
profileName: "p",
printerName: "",
avgDE: 1.0,
maxDE: 1.0,
rmsDE: 1.0,
patchCount: 1,
status: .good,
timestamp: Date(timeIntervalSince1970: 0)
)
do {
_ = try await store.append(record)
Issue.record("append() should propagate the load error")
} catch {
#expect(fm.fileExists(atPath: url.path))
if let data = try? Data(contentsOf: url),
let contents = String(data: data, encoding: .utf8) {
#expect(contents == badJSON)
} else {
Issue.record("Could not read preserved file")
}
}
}
@Test("CSV export quoting") @Test("CSV export quoting")
func csvQuoting() async throws { func csvQuoting() async throws {
let fm = FileManager.default let fm = FileManager.default
+1 -1
View File
@@ -39,7 +39,7 @@ def main():
def read_input(): def read_input():
line = read_line() line = read_line()
if not line: if line == "":
sys.exit(1) sys.exit(1)
return line.strip() return line.strip()
+6 -5
View File
@@ -93,7 +93,6 @@ final class Milestone4UITests: XCTestCase {
/// End-to-end handheld chartread with the mock fixture produces a /// End-to-end handheld chartread with the mock fixture produces a
/// canonical .ti3 and unlocks Stage 4. /// canonical .ti3 and unlocks Stage 4.
func testHandheldFixtureChartreadAndAverage() throws { func testHandheldFixtureChartreadAndAverage() throws {
try XCTSkipIf(true, "Full interactive chartread UI requires fixture timing tuning; skipped for CI stability. Core chartread/arteffact tests cover the model.")
reachStage3() reachStage3()
app.buttons["btnDetectInstruments"].click() app.buttons["btnDetectInstruments"].click()
@@ -113,19 +112,21 @@ final class Milestone4UITests: XCTestCase {
app.buttons["btnCalibrate"].click() app.buttons["btnCalibrate"].click()
// Trigger strip A. // Trigger strip A.
_ = waitFor("btnCalibrate", timeout: 20) _ = waitFor("btnTrigger", timeout: 20)
app.buttons["btnCalibrate"].click() app.buttons["btnTrigger"].click()
// Trigger strip B. // Trigger strip B.
_ = waitFor("btnCalibrate", timeout: 20) _ = waitFor("btnTrigger", timeout: 20)
app.buttons["btnCalibrate"].click() app.buttons["btnTrigger"].click()
// All strips read Done & Save appears. // All strips read Done & Save appears.
_ = waitFor("btnDoneRead", timeout: 20) _ = waitFor("btnDoneRead", timeout: 20)
app.buttons["btnDoneRead"].firstMatch.click() app.buttons["btnDoneRead"].firstMatch.click()
// Averaging panel appears with one pass snapshot. // Averaging panel appears with one pass snapshot.
_ = waitFor("chartreadAveragingPanel", timeout: 20)
_ = waitFor("passCounterBadge", timeout: 20) _ = waitFor("passCounterBadge", timeout: 20)
XCTAssertTrue(app.buttons["btnFinishAndAverage"].waitForExistence(timeout: 5))
XCTAssertTrue(app.buttons["btnFinishAndAverage"].isEnabled) XCTAssertTrue(app.buttons["btnFinishAndAverage"].isEnabled)
app.buttons["btnFinishAndAverage"].click() app.buttons["btnFinishAndAverage"].click()