Compare commits

..
Author SHA1 Message Date
gronod 460b0a1ffa Merge pull request 'M5 bugfixes — Stage 3 prompt stream, ProcessManager, profile/verification fixes (#50, #52)' (#53) from feat/52-m5-bugfixes into milestone/m5-profile 2026-09-09 14:21:42 +01:00
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 processId = ProcessID.targen(cleanBasename)
await ensureNotRunning(id: processId)
let events = processManager.events()
try await processManager.runStreaming(
id: processId,
@@ -121,6 +122,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("printtarg")
let processId = ProcessID.printtarg(cleanBasename)
await ensureNotRunning(id: processId)
let events = processManager.events()
try await processManager.runStreaming(
id: processId,
@@ -169,6 +171,19 @@ public struct ArgyllRunner: Sendable {
// 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 {
var exitCode: Int32?
var stdout: String
@@ -179,10 +194,15 @@ public struct ArgyllRunner: Sendable {
/// Drains the event stream until this child's `exit` event.
/// stdout is accumulated both per-line (logs) and verbatim (for
/// 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(
id processId: String,
events: AsyncStream<ProcessEvent>,
onLogBatch: (@Sendable ([String]) -> Void)?
onLogBatch: (@Sendable ([String]) -> Void)?,
flushPartialLines: Bool = false
) async -> CollectedRun {
var lines: [String] = []
var stdout = ""
@@ -198,6 +218,17 @@ public struct ArgyllRunner: Sendable {
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 {
guard event.id == processId else { continue }
switch event {
@@ -229,6 +260,12 @@ public struct ArgyllRunner: Sendable {
break
}
}
dotFlushTask?.cancel()
if let dotFlushTask {
_ = await dotFlushTask.value
}
return CollectedRun(exitCode: exitCode, stdout: stdout, stderr: stderr, lines: lines)
}
@@ -242,6 +279,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("instlist")
let processId = ProcessID.instlist
await ensureNotRunning(id: processId)
let events = processManager.events()
try await processManager.runStreaming(
id: processId,
@@ -301,6 +339,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("average")
let processId = ProcessID.average(config.basename)
await ensureNotRunning(id: processId)
let events = processManager.events()
try await processManager.runStreaming(
id: processId,
@@ -335,6 +374,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("colprof")
let processId = ProcessID.colprof(cleanBasename)
await ensureNotRunning(id: processId)
let events = processManager.events()
try await processManager.runStreaming(
id: processId,
@@ -342,7 +382,12 @@ public struct ArgyllRunner: Sendable {
arguments: args,
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 {
throw ArgyllRunnerError.colprofFailed(
@@ -368,10 +413,13 @@ public struct ArgyllRunner: Sendable {
///
/// Runs `applycal` captured and performs an in-place replace via
/// `{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(
config: ApplycalConfig
) async throws -> URL {
assert(!config.unapply, "runApplycal does not support unapply")
let inputURL = config.inputProfileURL
let cwd = inputURL.deletingLastPathComponent()
let binaryURL = binaryResolver.resolve("applycal")
@@ -383,6 +431,8 @@ public struct ArgyllRunner: Sendable {
// Remove any stale tmp from a previous crash.
try? fm.removeItem(at: tmpURL)
await ensureNotRunning(id: processId)
let outputConfig = ApplycalConfig(
calibrationPath: config.calibrationPath,
inputProfileURL: inputURL,
@@ -398,8 +448,11 @@ public struct ArgyllRunner: Sendable {
workingDirectory: cwd
)
guard result.exitCode == 0 else {
guard result.exitCode == 0, !Task.isCancelled else {
try? fm.removeItem(at: tmpURL)
if Task.isCancelled {
throw CancellationError()
}
throw ArgyllRunnerError.applycalFailed(
result.stderr.isEmpty
? "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 {
if fm.fileExists(atPath: inputURL.path) {
_ = try fm.replaceItemAt(inputURL, withItemAt: tmpURL)
@@ -441,6 +503,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("iccgamut")
let processId = ProcessID.iccgamut(stem: stem)
await ensureNotRunning(id: processId)
let events = processManager.events()
try await processManager.runStreaming(
id: processId,
@@ -480,6 +543,7 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("profcheck")
let processId = ProcessID.profcheck(ti3Path: ti3Path)
await ensureNotRunning(id: processId)
let events = processManager.events()
try await processManager.runStreaming(
id: processId,
@@ -547,11 +611,21 @@ public struct ArgyllRunner: Sendable {
let binaryURL = binaryResolver.resolve("chartread")
let processId = ProcessID.chartread(cleanBasename)
let processManager = self.processManager
let isXY = config.isXY
return AsyncStream { continuation in
let task = Task {
await ensureNotRunning(id: processId)
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 {
try await processManager.runStreaming(
id: processId,
@@ -582,20 +656,22 @@ public struct ArgyllRunner: Sendable {
switch event {
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
if classified.isRemoveSheetNotice {
continuation.yield(.removeSheetNotice)
}
if classified.sheetNumber != nil || classified.alignmentPatch != nil {
continuation.yield(.prompt(classified))
} else if state != previousOrContinuationState(state, classified) {
// Only emit prompt when the state meaningfully changes.
continuation.yield(.prompt(classified))
} else if state == .tablePlaceSheet || state == .tableAlign {
// Continuation lines in table states are still prompts.
continuation.yield(.prompt(classified))
} else if classified.requestedWarningKey != nil {
let shouldPrompt =
classified.sheetNumber != nil
|| classified.alignmentPatch != nil
|| classified.requestedWarningKey != nil
|| classified.state != previous
|| classified.isTableContinuation
if shouldPrompt {
continuation.yield(.prompt(classified))
}
@@ -632,6 +708,11 @@ public struct ArgyllRunner: Sendable {
}
}
if Task.isCancelled {
continuation.finish()
return
}
let canonical = cwd.appendingPathComponent("\(cleanBasename).ti3")
if let code = exitCode, code == 0 {
if FileManager.default.fileExists(atPath: canonical.path) {
@@ -647,15 +728,13 @@ public struct ArgyllRunner: Sendable {
continuation.onTermination = { _ in
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.
public func sendChartreadInput(basename: String, input: ChartreadInput) async throws {
let cleanBasename = try PathSecurity.sanitizeBasename(basename)
@@ -665,17 +744,14 @@ public struct ArgyllRunner: Sendable {
/// 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) {
let cleanBasename = try? PathSecurity.sanitizeBasename(basename)
guard let cleanBasename else { return }
let processId = ProcessID.chartread(cleanBasename)
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)
}
}
@@ -153,7 +153,7 @@ public enum ChartreadClassifier {
let phrases = [
"'d' if/when done", "d to finish/save", "all strips/patches read",
"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) }) {
return ChartreadClassifyResult(state: .allStripsRead)
@@ -163,20 +163,28 @@ public enum ChartreadClassifier {
// 7. Warnings / prompts needing a key.
private static func warning(text: String, previous: ChartreadState) -> ChartreadClassifyResult? {
let lower = text
let warningSignals = [
"(warning)", "use it anyway", "seem to have read strip pass",
"unexpected response", "seem to have read", "misread",
"try again", "do you want to"
"(warning)", "use it anyway", "seem to have read strip",
"unexpected response", "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?
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.
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"
} 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"
}
@@ -195,12 +203,14 @@ public enum ChartreadClassifier {
!hasLocate
else { return nil }
if lowercased.contains("hit any key to continue")
|| lowercased.contains("hit space to continue")
|| lowercased.contains("calibration")
|| lowercased.contains("calibrate")
if lowercased.contains("calibrat")
|| lowercased.contains("white reference")
|| 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 nil
@@ -209,11 +219,29 @@ public enum ChartreadClassifier {
// 9. Awaiting strip.
private static func awaitingStrip(text: String, previous: ChartreadState) -> ChartreadClassifyResult? {
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 = [
"hit ... read ... strip", "ready to read", "read ... strip ... key",
"hit any key to read", "ready to read strip", "hit a key to read",
"press any key to read", "read strip"
"ready to read",
"hit any key to read",
"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 }
return ChartreadClassifyResult(state: .awaitingStrip)
}
@@ -228,16 +256,27 @@ public enum ChartreadClassifier {
// 11. Error.
private static func error(text: String, previous: ChartreadState) -> ChartreadClassifyResult? {
let phrases = ["error", "too fast", "too slow", "misread", "failed to read", "failed"]
// 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 }
let lower = text.lowercased()
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 nil
}
}
@@ -35,6 +35,18 @@ public struct ProcessLineDecoder: Sendable {
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 {
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.
/// - `exit` is emitted exactly once per child, and only after both
/// output pipes reach EOF so no buffered output is lost on fast
/// exits or kills.
/// - `kill` drops the stdin handle so writers fail fast.
/// exits or kills. If EOFs never arrive, a watchdog finalizes.
/// - `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 static let rowColorsPrefix = "ROW_COLORS_JSON: "
@@ -92,12 +95,18 @@ public actor ProcessManager {
/// pipes have also reached EOF.
var pendingExitCode: Int32?
var finalized = false
/// Watchdog that forces finalization if EOFs never arrive.
var finalizeTask: Task<Void, Never>?
}
private var children: [String: RunningChild] = [:]
/// Processes owned by `runCaptured` (dup detection + kill support).
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.
public var runningIDs: [String] { Array(children.keys) + captured.keys }
@@ -105,6 +114,14 @@ public actor ProcessManager {
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)
/// Spawns a streaming child. Returns after spawn; callers wait for
@@ -142,14 +159,6 @@ public actor ProcessManager {
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 stderrHandle = stderrPipe.fileHandleForReading
stdoutHandle.readabilityHandler = { [weak self] handle in
@@ -167,6 +176,15 @@ public actor ProcessManager {
guard let self else { return }
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)
@@ -198,41 +216,114 @@ public actor ProcessManager {
"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
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 {
try process.run()
} catch {
_ = box.resume(with: -1)
captured.removeValue(forKey: id)
preKillHooks.removeValue(forKey: id)
emit(.error(id: id, message: error.localizedDescription))
throw ProcessError.spawnFailed("\(binary.path): \(error.localizedDescription)")
}
async let outData = Task.detached {
stdoutPipe.fileHandleForReading.readDataToEndOfFile()
}.value
async let errData = Task.detached {
stderrPipe.fileHandleForReading.readDataToEndOfFile()
}.value
// Close the parent write ends so readDataToEndOfFile() gets EOF
// as soon as the child exits; the child still has its own copies.
try? stdoutPipe.fileHandleForWriting.close()
try? stderrPipe.fileHandleForWriting.close()
let code = await withCheckedContinuation { continuation in
process.terminationHandler = { proc in
continuation.resume(returning: proc.terminationStatus)
return await withTaskCancellationHandler {
async let outData = Task.detached {
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
@@ -255,39 +346,89 @@ public actor ProcessManager {
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
/// Terminates a child. The `exit` event still fires exactly once.
/// stdin is dropped immediately so writers fail fast (docs/03 rule 7).
public func kill(id: String) {
/// Terminates a child. First runs any registered pre-kill hook, then
/// drops stdin and signals the process. For streaming children the
/// `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] {
try? child.stdin?.close()
child.stdin = nil
children[id] = child
if child.process.isRunning {
child.process.terminate()
} else {
Task { await self.didTerminate(id: id, code: child.process.terminationStatus) }
} else if child.pendingExitCode == nil {
// The process already exited but `didTerminate` has not
// run; synthesize it so `maybeFinalize` can fire.
didTerminate(id: id, code: child.process.terminationStatus)
}
return
}
if let process = captured[id] {
if process.isRunning { process.terminate() }
if captured.removeValue(forKey: id) != nil {
emit(.exit(id: id, code: process.terminationStatus))
}
// Do not emit `.exit` here; `runCaptured` emits the real code
// after the process reaps.
return
}
}
/// Terminates every running child; returns how many were signaled
/// (`kill_all_processes`, docs/03). Mandatory on app exit (#147/#149).
@discardableResult
public func killAll() -> Int {
let ids = Array(children.keys) + Array(captured.keys)
for id in ids { kill(id: id) }
public func killAll() async -> Int {
let ids = runningIDs
for id in ids { await kill(id: id) }
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
private func childEnvironment(extra: [String: String]) -> [String: String] {
@@ -339,6 +480,16 @@ public actor ProcessManager {
child.pendingExitCode = code
try? child.stdin?.close()
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
maybeFinalize(id: id)
}
@@ -351,8 +502,12 @@ public actor ProcessManager {
child.stdoutEOF, child.stderrEOF,
!child.finalized
else { return }
child.finalized = true
child.finalizeTask?.cancel()
child.finalizeTask = nil
children.removeValue(forKey: id)
preKillHooks.removeValue(forKey: id)
// Flush unterminated tail lines.
if var decoder = Optional(child.stdoutDecoder),
@@ -369,4 +524,20 @@ public actor ProcessManager {
}
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.
///
/// A drift alert triggers when there are at least two `poor` records on
/// distinct calendar days, or two `poor` records at least one hour apart.
/// A drift alert triggers when the most recent chronologically consecutive
/// 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 {
/// Returns an alert message, or `nil` when no consecutive breach exists.
public static func compute(from records: [VerificationRecord]) -> String? {
let poor = records
.filter { $0.status == .poor }
.sorted { $0.timestamp < $1.timestamp }
// Work in chronological order.
let chronological = records.sorted { $0.timestamp < $1.timestamp }
guard poor.count >= 2 else { return nil }
for i in 0..<poor.count {
for j in (i + 1)..<poor.count {
let a = poor[i]
let b = poor[j]
let sameDay = Calendar.utc.isDate(a.timestamp, inSameDayAs: b.timestamp)
let oneHour = b.timestamp.timeIntervalSince(a.timestamp) >= 3600
if !sameDay || oneHour {
return "Drift alert: poor results between \(a.id) and \(b.id)."
}
// Build the longest suffix of consecutive `.poor` records.
// Non-poor records break the run, so we stop at the first non-poor
// encountered from the end.
var run: [VerificationRecord] = []
for record in chronological.reversed() {
if record.status == .poor {
run.insert(record, at: 0)
} else {
break
}
}
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
}
}
@@ -33,10 +33,32 @@ public enum ProfileInstallError: LocalizedError, Equatable, Sendable {
/// Installs an ICC/ICM profile into the OS colour store.
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
/// `/Library/ColorSync/Profiles`. Always copies, never moves.
public static func install(config: InstallProfileConfig) throws -> InstallProfileResult {
let fm = FileManager.default
public static func install(
config: InstallProfileConfig,
fileManager: FileManager = .default
) throws -> InstallProfileResult {
let fm = fileManager
// Source validation.
let sourceURL = config.sourceURL
@@ -55,38 +77,37 @@ public enum ProfileInstaller {
throw ProfileInstallError.sourceTooSmall
}
// Stem security.
let stem = sourceURL.deletingPathExtension().lastPathComponent
guard !stem.contains("..") && !stem.contains("/") && !stem.contains("\\") else {
throw ProfileInstallError.unsafeStem(stem)
}
try validateSourceURL(sourceURL)
// Destination directory.
let destDir: URL
if config.options.preferSystem {
destDir = URL(fileURLWithPath: "/Library/ColorSync/Profiles")
} else {
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")
let destURL = try resolveDestinationURL(for: config, fileManager: fm)
try? fm.createDirectory(
at: destURL.deletingLastPathComponent(),
withIntermediateDirectories: true
)
// Collision resolution.
let destExists = fm.fileExists(atPath: destURL.path)
if destExists {
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 {
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(
from: sourceURL,
to: renamedURL,
options: config.options,
fileManager: fm,
overwritten: false,
renamed: true
)
@@ -102,19 +123,53 @@ public enum ProfileInstaller {
from: sourceURL,
to: destURL,
options: config.options,
overwritten: destExists,
fileManager: fm,
overwritten: 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(
from sourceURL: URL,
to destURL: URL,
options: InstallProfileOptions,
fileManager: FileManager,
overwritten: Bool,
renamed: Bool
) throws -> InstallProfileResult {
let fm = FileManager.default
let fm = fileManager
let tmpURL = destURL.appendingPathExtension("iccery-install.tmp")
// Remove stale tmp.
@@ -123,6 +178,13 @@ public enum ProfileInstaller {
do {
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) {
_ = try fm.replaceItemAt(destURL, withItemAt: tmpURL)
} else {
@@ -135,6 +197,10 @@ public enum ProfileInstaller {
if destURL.path.hasPrefix("/Library/") && !fm.fileExists(atPath: destURL.path) {
throw ProfileInstallError.systemRequiresAdminRights
}
if let installError = error as? ProfileInstallError {
throw installError
}
throw ProfileInstallError.copyFailed(error.localizedDescription)
}
@@ -57,10 +57,12 @@ public actor VerificationHistoryStore {
/// Appends a record, trims to capacity, and writes atomically.
///
/// Returns the trimmed list, or `nil` if a write error occurs so the
/// caller can surface the failure without replacing the in-memory list.
/// Loads the existing history first and propagates any load error so an
/// unparseable file is never overwritten.
@discardableResult
public func append(_ record: VerificationRecord) throws -> [VerificationRecord] {
try load()
var updated = records
updated.append(record)
if updated.count > capacity {
@@ -305,6 +305,10 @@ final class MeasurementWorkflowViewModel {
environment.runner.cancelChartread(basename: basename, isXY: selectedInstrument.isXY)
chartreadTask?.cancel()
isChartreadRunning = false
chartreadState = .idle
currentPrompt = nil
requestedWarningKey = nil
showRemoveSheetNotice = false
}
func sendWarningKey(_ key: String) {
+43 -24
View File
@@ -81,6 +81,15 @@ final class ProfileWorkflowViewModel {
init(wizard: WizardViewModel, environment: AppEnvironment) {
self.wizard = wizard
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
@@ -206,6 +215,7 @@ final class ProfileWorkflowViewModel {
calibrationPath: self.calibrationFile,
inputProfileURL: url
)
assert(!applyConfig.unapply, "applycal unapply is not supported in v2.0")
finalProfileURL = try await runner.runApplycal(config: applyConfig)
self.colprofLog.append("Calibration embedded: \(self.calibrationFile)")
}
@@ -256,7 +266,15 @@ final class ProfileWorkflowViewModel {
// MARK: - Stage 5: verify profile
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() {
@@ -333,10 +351,11 @@ final class ProfileWorkflowViewModel {
let timestamp = Date()
let id = "vr-\(Int(timestamp.timeIntervalSince1970))-\(Self.nextSeq())"
let printerName = wizard.printerName?.isEmpty == false ? wizard.printerName! : "Unknown"
return VerificationRecord(
id: id,
profileName: createdProfileURL?.lastPathComponent ?? wizard.basename,
printerName: wizard.printerName ?? "",
printerName: printerName,
avgDE: avg,
maxDE: max,
rmsDE: rms,
@@ -381,24 +400,32 @@ final class ProfileWorkflowViewModel {
openColorPanel: settings.openColorPanelAfterInstall
)
let destURL = installDestination(for: sourceURL, options: options)
let collision = FileManager.default.fileExists(atPath: destURL.path)
do {
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 {
pendingInstallOptions = options
installCollisionMessage = "A profile named \(destURL.lastPathComponent) already exists."
showingInstallCollision = true
return
if collision && settings.askBeforeOverwriteProfile {
pendingInstallOptions = options
installCollisionMessage = "A profile named \(destURL.lastPathComponent) already exists."
showingInstallCollision = true
return
}
runInstall(sourceURL: sourceURL, options: options)
} catch {
wizard.showNotice(
"Install failed: \(error.localizedDescription)",
kind: .error
)
}
runInstall(sourceURL: sourceURL, options: options)
}
func resolveInstallCollision(policy: ProfileCollisionPolicy) {
showingInstallCollision = false
guard let sourceURL = createdProfileURL,
var options = pendingInstallOptions else { return }
options.collisionPolicy = policy
if policy == .cancel {
installResult = InstallProfileResult(
destPath: "",
@@ -410,20 +437,12 @@ final class ProfileWorkflowViewModel {
)
return
}
runInstall(sourceURL: sourceURL, options: options)
}
private func installDestination(for sourceURL: URL, options: InstallProfileOptions) -> URL {
let stem = sourceURL.deletingPathExtension().lastPathComponent
let fm = FileManager.default
let destDir: URL
if options.preferSystem {
destDir = URL(fileURLWithPath: "/Library/ColorSync/Profiles")
} else {
destDir = fm.homeDirectoryForCurrentUser
.appendingPathComponent("Library/ColorSync/Profiles")
options.collisionPolicy = policy
if policy == .overwrite {
options.forceOverwrite = true
}
return destDir.appendingPathComponent("\(stem).icc")
runInstall(sourceURL: sourceURL, options: options)
}
private func runInstall(sourceURL: URL, options: InstallProfileOptions) {
+5 -12
View File
@@ -210,7 +210,9 @@ struct Stage3View: View {
.accessibilityIdentifier("btnCalibrate")
case .awaitingStrip:
Button("Trigger") { model.calibrate() }
.accessibilityIdentifier("btnCalibrate")
.accessibilityIdentifier("btnTrigger")
Button("Done & Save") { model.doneAndSave() }
.accessibilityIdentifier("btnDoneReadEarly")
case .tablePlaceSheet, .tableAlign, .promptContinue, .warning:
Button(continueTitle) { model.accept() }
.accessibilityIdentifier("btnAccept")
@@ -224,16 +226,6 @@ struct Stage3View: View {
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() }
.accessibilityIdentifier("btnCancel")
}
@@ -374,7 +366,7 @@ struct Stage3View: View {
Button("Finish & Average") {
model.finishAndAverage()
}
.disabled(!model.isFinished || model.isFinishing)
.disabled(!model.canFinish || model.isFinishing)
.accessibilityIdentifier("btnFinishAndAverage")
}
@@ -386,6 +378,7 @@ struct Stage3View: View {
}
.padding(16)
.background(Theme.panel)
.accessibilityElement(children: .contain)
.accessibilityIdentifier("chartreadAveragingPanel")
}
}
+1
View File
@@ -18,6 +18,7 @@ struct Stage4View: View {
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.background(Theme.background)
.onAppear { model.restoreCreatedProfileURL() }
}
// MARK: - Header
+5 -2
View File
@@ -21,7 +21,10 @@ struct Stage5View: View {
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.background(Theme.background)
.onAppear { model.loadHistory() }
.onAppear {
model.restoreCreatedProfileURL()
model.loadHistory()
}
.alert("Install profile", isPresented: $model.showingInstallCollision) {
Button("Overwrite", role: .destructive) {
model.resolveInstallCollision(policy: .overwrite)
@@ -70,7 +73,7 @@ struct Stage5View: View {
.accessibilityIdentifier("driftAlert")
}
if let warning = model.profcheckWarning, !warning.isEmpty, model.driftAlert == nil {
if let warning = model.profcheckWarning, !warning.isEmpty {
Text("\(warning)")
.font(.caption)
.padding(.horizontal, 8)
@@ -15,16 +15,16 @@ struct ApplycalArgsTests {
#expect(args == ["-v", "-a", "/tmp/cal.cal", "/tmp/profile.icc"])
}
@Test("Unapply is never sent from build")
func unapplyNotEmitted() throws {
@Test("Unapply is emitted when the caller explicitly sets it")
func unapplyEmittedWhenConfigSet() throws {
let config = ApplycalConfig(
calibrationPath: "/tmp/cal.cal",
inputProfileURL: URL(fileURLWithPath: "/tmp/profile.icc"),
unapply: true
)
let args = try ApplycalArgs.build(config: config)
// Builder intentionally emits -u because config can set it, but
// the UI layer never passes unapply: true in v2.0.
// Builder emits -u only when the caller explicitly sets unapply.
// The UI layer never passes unapply: true in v2.0.
#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)
}
@Test("Non-poor results do not trigger")
@Test("Non-poor records do not trigger")
func nonPoor() {
let records = [
record(avg: 1.0, at: 0),
@@ -47,6 +47,48 @@ struct DriftAlertTests {
#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 {
VerificationRecord(
id: "vr-\(Int(offset))",
+140 -44
View File
@@ -2,42 +2,165 @@ import Foundation
import Testing
@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")
struct ProfileInstallerTests {
@Test("Copies .icc to user ColorSync folder")
func userInstall() throws {
private func makeTempDir() throws -> URL {
let fm = FileManager.default
let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString)
try fm.createDirectory(at: tmp, withIntermediateDirectories: true)
return tmp
}
let source = tmp.appendingPathComponent("test.icc")
let iccData = Data(repeating: 0, count: 256)
try iccData.write(to: source)
private func makeSource(
at dir: URL,
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
// is not practical; instead exercise Core validation on a
// temp-only path via the file URL safety checks and the public
// install against a writable system-like path is tested below.
let result = try ProfileInstaller.install(
config: InstallProfileConfig(sourceURL: source),
fileManager: testFM
)
#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 {
_ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: unsafe))
_ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: sourceURL))
Issue.record("Expected unsafeStem error")
} catch let error as ProfileInstallError {
if case .unsafeStem = error { } else { Issue.record("Expected unsafeStem, got \(error)") }
} catch {
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 {
_ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: small))
_ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: source))
Issue.record("Expected sourceTooSmall error")
} catch let error as ProfileInstallError {
if case .sourceTooSmall = error { } else { Issue.record("Expected sourceTooSmall, got \(error)") }
@@ -45,31 +168,4 @@ struct ProfileInstallerTests {
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")
func csvQuoting() async throws {
let fm = FileManager.default
+1 -1
View File
@@ -39,7 +39,7 @@ def main():
def read_input():
line = read_line()
if not line:
if line == "":
sys.exit(1)
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
/// canonical .ti3 and unlocks Stage 4.
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()
app.buttons["btnDetectInstruments"].click()
@@ -113,19 +112,21 @@ final class Milestone4UITests: XCTestCase {
app.buttons["btnCalibrate"].click()
// Trigger strip A.
_ = waitFor("btnCalibrate", timeout: 20)
app.buttons["btnCalibrate"].click()
_ = waitFor("btnTrigger", timeout: 20)
app.buttons["btnTrigger"].click()
// Trigger strip B.
_ = waitFor("btnCalibrate", timeout: 20)
app.buttons["btnCalibrate"].click()
_ = waitFor("btnTrigger", timeout: 20)
app.buttons["btnTrigger"].click()
// All strips read Done & Save appears.
_ = waitFor("btnDoneRead", timeout: 20)
app.buttons["btnDoneRead"].firstMatch.click()
// Averaging panel appears with one pass snapshot.
_ = waitFor("chartreadAveragingPanel", timeout: 20)
_ = waitFor("passCounterBadge", timeout: 20)
XCTAssertTrue(app.buttons["btnFinishAndAverage"].waitForExistence(timeout: 5))
XCTAssertTrue(app.buttons["btnFinishAndAverage"].isEnabled)
app.buttons["btnFinishAndAverage"].click()