From 8af3c3599c1bd48b77803f3062b07e5bde10b9be Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 12:07:47 +0300 Subject: [PATCH 1/3] fix: resolve ConfigValidator SIGTRAP crash from MainActor isolation conflict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConfigValidator was a `final class` without explicit isolation, inheriting `@MainActor` from `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor` build setting. It dispatched work to background queue `com.pine.config-validation`, causing runtime assertion failure (dispatch_assert_queue_fail → SIGTRAP). Fix: Extract heavy validation work into `nonisolated enum ConfigValidationWorker` (same pattern as `GitFetcher`), make `ConfigValidator` explicitly `@MainActor`, replace DispatchQueue-based debounce with Task.sleep. All supporting types (ValidatorKind, ValidatorDetector, ToolAvailability, ValidatorOutputParser, BuiltinValidator) marked `nonisolated` to allow background access. --- Pine/ConfigValidator.swift | 240 ++++++++++++++------------- PineTests/ConfigValidatorTests.swift | 85 +++++++++- 2 files changed, 203 insertions(+), 122 deletions(-) diff --git a/Pine/ConfigValidator.swift b/Pine/ConfigValidator.swift index 2281dfa..3191e33 100644 --- a/Pine/ConfigValidator.swift +++ b/Pine/ConfigValidator.swift @@ -10,14 +10,14 @@ import Foundation // MARK: - Models /// Severity of a validation diagnostic. -enum ValidationSeverity: Sendable, Equatable { +nonisolated enum ValidationSeverity: Sendable, Equatable { case error case warning case info } /// A single validation diagnostic tied to a line in the file. -struct ValidationDiagnostic: Sendable, Equatable, Identifiable { +nonisolated struct ValidationDiagnostic: Sendable, Equatable, Identifiable { let id = UUID() let line: Int let column: Int? @@ -36,7 +36,7 @@ struct ValidationDiagnostic: Sendable, Equatable, Identifiable { } /// The type of config validator to use for a given file. -enum ValidatorKind: Sendable, Equatable { +nonisolated enum ValidatorKind: Sendable, Equatable { case yamllint case terraform case shellcheck @@ -66,7 +66,7 @@ enum ValidatorKind: Sendable, Equatable { // MARK: - Validator Detection /// Determines which validator to use based on file extension or name. -enum ValidatorDetector { +nonisolated enum ValidatorDetector { static func detect(for url: URL) -> ValidatorKind? { let ext = url.pathExtension.lowercased() let name = url.lastPathComponent.lowercased() @@ -94,9 +94,10 @@ enum ValidatorDetector { // MARK: - Tool Availability /// Checks whether a command-line tool is available via `which`. -enum ToolAvailability { +nonisolated enum ToolAvailability { /// Cached availability results to avoid repeated `which` calls. - private static var cache: [String: String?] = [:] + /// nonisolated(unsafe): protected by `lock` — all reads and writes go through lock/unlock. + nonisolated(unsafe) private static var cache: [String: String?] = [:] private static let lock = NSLock() /// Returns the full path to the tool if installed, nil otherwise. @@ -163,7 +164,7 @@ enum ToolAvailability { // MARK: - Output Parsers /// Parses output from various config validators into diagnostics. -enum ValidatorOutputParser { +nonisolated enum ValidatorOutputParser { // MARK: - yamllint @@ -339,7 +340,7 @@ enum ValidatorOutputParser { /// Built-in regex-based validators that work without external tools. /// These provide basic validation when CLI tools (yamllint, hadolint, etc.) are not installed. -enum BuiltinValidator { +nonisolated enum BuiltinValidator { // Cached regex for detecting unquoted variables in shell test expressions. // swiftlint:disable:next force_try @@ -584,102 +585,22 @@ enum BuiltinValidator { } } -// MARK: - ConfigValidator - -/// Runs external config validators and produces diagnostics. -/// Falls back to built-in regex-based validators when external tools are not installed. -/// Designed to be called from a background queue with debouncing. -@Observable -final class ConfigValidator { - - /// Current diagnostics for the active file. - private(set) var diagnostics: [ValidationDiagnostic] = [] - - /// Whether validation is currently running. - private(set) var isValidating = false - - /// The validator kind for the current file, if any. - private(set) var activeValidator: ValidatorKind? - - /// Whether the required tool is available. - private(set) var toolAvailable = false - - /// Debounce interval in seconds. - static let debounceInterval: TimeInterval = 0.3 - - /// Serial queue for validation work. - private let queue = DispatchQueue(label: "com.pine.config-validation", qos: .utility) - - /// Lock protecting the generation token, which is read/written from both - /// the main thread (clear()) and the background queue (runValidation()). - private let generationLock = NSLock() - - /// Generation token to discard stale results. - private var generation: UInt64 = 0 - - /// Debounce work item. - private var debounceWorkItem: DispatchWorkItem? - - /// Thread-safe read of the current generation. - private func currentGeneration() -> UInt64 { - generationLock.lock() - defer { generationLock.unlock() } - return generation - } - - /// Thread-safe increment-and-return of the generation. - private func nextGeneration() -> UInt64 { - generationLock.lock() - defer { generationLock.unlock() } - generation &+= 1 - return generation - } - - /// Validates the given file content, debounced. - /// - Parameters: - /// - url: The file URL (used for extension detection and temp file creation). - /// - content: The current file content. - func validate(url: URL, content: String) { - debounceWorkItem?.cancel() - - guard let kind = ValidatorDetector.detect(for: url) else { - DispatchQueue.main.async { [weak self] in - self?.diagnostics = [] - self?.activeValidator = nil - self?.toolAvailable = false - } - return - } - - let workItem = DispatchWorkItem { [weak self] in - self?.runValidation(url: url, content: content, kind: kind) - } - debounceWorkItem = workItem - queue.asyncAfter(deadline: .now() + Self.debounceInterval, execute: workItem) - } - - /// Clears all diagnostics (e.g. when switching tabs). - func clear() { - debounceWorkItem?.cancel() - _ = nextGeneration() - DispatchQueue.main.async { [weak self] in - self?.diagnostics = [] - self?.activeValidator = nil - self?.toolAvailable = false - self?.isValidating = false - } - } - - // MARK: - Private - - private func runValidation(url: URL, content: String, kind: ValidatorKind) { - let currentGen = nextGeneration() - - DispatchQueue.main.async { [weak self] in - self?.isValidating = true - self?.activeValidator = kind - } - +// MARK: - ConfigValidationWorker + +/// Namespace for config validation work that runs on background threads. +/// Deliberately **not** `@MainActor` so closures inside `DispatchQueue.global().async` +/// do not inherit MainActor isolation — prevents `dispatch_assert_queue_fail` +/// crash under Swift 6 strict concurrency. +/// Marked `nonisolated` to opt out of `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`. +nonisolated enum ConfigValidationWorker { + + /// Runs validation on the current thread (expected to be called from a background queue). + /// Returns parsed diagnostics and whether an external tool was available. + static func runValidation( + url: URL, + content: String, + kind: ValidatorKind + ) -> (diagnostics: [ValidationDiagnostic], toolAvailable: Bool) { // Check tool availability let toolPath = ToolAvailability.path(for: kind.toolName) let hasExternalTool = toolPath != nil @@ -697,8 +618,6 @@ final class ConfigValidator { let result = runTool(toolPath: toolPath, kind: kind, filePath: tempFile.path) - guard currentGeneration() == currentGen else { return } - switch kind { case .yamllint: parsed = ValidatorOutputParser.parseYamllint(result) @@ -714,8 +633,6 @@ final class ConfigValidator { } } - guard currentGeneration() == currentGen else { return } - // Fall back to built-in validation only when external tool is not installed. // If external tool is installed and returned empty output, the file is valid. if parsed.isEmpty && !hasExternalTool { @@ -731,17 +648,10 @@ final class ConfigValidator { } } - guard currentGeneration() == currentGen else { return } - - DispatchQueue.main.async { [weak self] in - guard self?.currentGeneration() == currentGen else { return } - self?.diagnostics = parsed - self?.toolAvailable = hasExternalTool - self?.isValidating = false - } + return (parsed, hasExternalTool) } - private func runTool(toolPath: String, kind: ValidatorKind, filePath: String) -> String { + static func runTool(toolPath: String, kind: ValidatorKind, filePath: String) -> String { let process = Process() process.executableURL = URL(fileURLWithPath: toolPath) @@ -790,3 +700,99 @@ final class ConfigValidator { return output.isEmpty ? errOutput : output } } + +// MARK: - ConfigValidator + +/// Runs external config validators and produces diagnostics. +/// Falls back to built-in regex-based validators when external tools are not installed. +/// Explicitly `@MainActor` — all UI state lives here. Heavy validation work is +/// dispatched to `ConfigValidationWorker` (nonisolated) to avoid +/// `dispatch_assert_queue_fail` crashes under `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`. +@MainActor +@Observable +final class ConfigValidator { + + /// Current diagnostics for the active file. + private(set) var diagnostics: [ValidationDiagnostic] = [] + + /// Whether validation is currently running. + private(set) var isValidating = false + + /// The validator kind for the current file, if any. + private(set) var activeValidator: ValidatorKind? + + /// Whether the required tool is available. + private(set) var toolAvailable = false + + /// Debounce interval in seconds. + nonisolated static let debounceInterval: TimeInterval = 0.3 + + /// Generation token to discard stale results. + private var generation: UInt64 = 0 + + /// Debounce task handle. + private var debounceTask: Task? + + /// Increments and returns the new generation token. + private func nextGeneration() -> UInt64 { + generation &+= 1 + return generation + } + + /// Validates the given file content, debounced. + /// - Parameters: + /// - url: The file URL (used for extension detection and temp file creation). + /// - content: The current file content. + func validate(url: URL, content: String) { + debounceTask?.cancel() + + guard let kind = ValidatorDetector.detect(for: url) else { + diagnostics = [] + activeValidator = nil + toolAvailable = false + return + } + + let currentGen = nextGeneration() + + debounceTask = Task { [weak self] in + // Debounce + try? await Task.sleep(for: .milliseconds(300)) + guard !Task.isCancelled, let self else { return } + + self.runValidation(url: url, content: content, kind: kind, expectedGen: currentGen) + } + } + + /// Clears all diagnostics (e.g. when switching tabs). + func clear() { + debounceTask?.cancel() + _ = nextGeneration() + diagnostics = [] + activeValidator = nil + toolAvailable = false + isValidating = false + } + + // MARK: - Private + + private func runValidation(url: URL, content: String, kind: ValidatorKind, expectedGen: UInt64) { + guard generation == expectedGen else { return } + + isValidating = true + activeValidator = kind + + let capturedGen = expectedGen + + Task.detached { + let result = ConfigValidationWorker.runValidation(url: url, content: content, kind: kind) + + await MainActor.run { [weak self] in + guard let self, self.generation == capturedGen else { return } + self.diagnostics = result.diagnostics + self.toolAvailable = result.toolAvailable + self.isValidating = false + } + } + } +} diff --git a/PineTests/ConfigValidatorTests.swift b/PineTests/ConfigValidatorTests.swift index 1449f6b..2ac8102 100644 --- a/PineTests/ConfigValidatorTests.swift +++ b/PineTests/ConfigValidatorTests.swift @@ -666,9 +666,9 @@ struct ConfigValidatorTests { #expect(results.filter { $0.message.contains("backtick") }.isEmpty) } - // MARK: - ConfigValidator generation lock + // MARK: - ConfigValidator generation token - @Test func validator_generationLock_initialState() { + @Test func validator_generationToken_initialState() { let validator = ConfigValidator() #expect(validator.diagnostics.isEmpty) #expect(validator.isValidating == false) @@ -676,11 +676,86 @@ struct ConfigValidatorTests { #expect(validator.toolAvailable == false) } - @Test func validator_clear_resetsAll() { + @Test func validator_clear_resetsAll_synchronously() { let validator = ConfigValidator() validator.clear() - // After clear, diagnostics should be empty (async) - #expect(validator.activeValidator == nil || true) // async, can't assert timing + // clear() is now synchronous on @MainActor — state is immediately reset + #expect(validator.diagnostics.isEmpty) + #expect(validator.activeValidator == nil) + #expect(validator.toolAvailable == false) + #expect(validator.isValidating == false) + } + + // MARK: - ConfigValidationWorker (nonisolated) + + @Test func worker_builtinYAML_tabIndentation() { + let url = URL(fileURLWithPath: "/tmp/test.yml") + let content = "key: value\n\tindented: bad\n" + let result = ConfigValidationWorker.runValidation(url: url, content: content, kind: .yamllint) + if result.toolAvailable { + // External yamllint handles validation — result depends on host config + // Just verify no crash (the core fix being tested) + } else { + // Built-in validator catches tab indentation + let tabErrors = result.diagnostics.filter { $0.message.contains("tab") } + #expect(!tabErrors.isEmpty) + } + } + + @Test func worker_builtinDockerfile_missingFrom() { + let url = URL(fileURLWithPath: "/tmp/Dockerfile") + let content = "RUN echo hello\n" + let result = ConfigValidationWorker.runValidation(url: url, content: content, kind: .hadolint) + // Built-in validator should report missing FROM (if hadolint not installed) + if !result.toolAvailable { + let fromErrors = result.diagnostics.filter { $0.message.contains("FROM") } + #expect(!fromErrors.isEmpty) + } + } + + @Test func worker_builtinShell_backticks() { + let url = URL(fileURLWithPath: "/tmp/test.sh") + let content = "result=`date`\n" + let result = ConfigValidationWorker.runValidation(url: url, content: content, kind: .shellcheck) + if !result.toolAvailable { + let backtickInfo = result.diagnostics.filter { $0.severity == .info } + #expect(!backtickInfo.isEmpty) + } + } + + @Test func worker_terraform_noBuiltinFallback() { + let url = URL(fileURLWithPath: "/tmp/main.tf") + let content = "resource \"null_resource\" \"test\" {}\n" + let result = ConfigValidationWorker.runValidation(url: url, content: content, kind: .terraform) + if !result.toolAvailable { + // No built-in terraform validation — empty diagnostics expected + #expect(result.diagnostics.isEmpty) + } + } + + @Test func worker_validYAML_noDiagnostics() { + let url = URL(fileURLWithPath: "/tmp/valid.yml") + let content = "key: value\nlist:\n - item1\n - item2\n" + let result = ConfigValidationWorker.runValidation(url: url, content: content, kind: .yamllint) + if !result.toolAvailable { + #expect(result.diagnostics.isEmpty) + } + } + + @Test func worker_canBeCalledFromBackgroundThread() async { + // This test verifies that ConfigValidationWorker can be called from + // any isolation domain without crashing — the core fix for the SIGTRAP. + let url = URL(fileURLWithPath: "/tmp/test.yml") + let content = "key: value\n\tindented: bad\n" + + let result = await Task.detached { + ConfigValidationWorker.runValidation(url: url, content: content, kind: .yamllint) + }.value + + if !result.toolAvailable { + let tabErrors = result.diagnostics.filter { $0.message.contains("tab") } + #expect(!tabErrors.isEmpty) + } } // MARK: - BuiltinValidator Dockerfile edge cases From 3c1dc1e3dce1c9fd32b061fcf802f23b66211355 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 12:15:40 +0300 Subject: [PATCH 2/3] fix: add nonisolated to InlineDiffProvider enum --- Pine/InlineDiffProvider.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pine/InlineDiffProvider.swift b/Pine/InlineDiffProvider.swift index dbf0302..8c560a2 100644 --- a/Pine/InlineDiffProvider.swift +++ b/Pine/InlineDiffProvider.swift @@ -93,7 +93,7 @@ struct DeletedLinesBlock: Equatable, Sendable { // MARK: - InlineDiffProvider /// Provides diff hunk parsing and accept/revert operations for editor files. -enum InlineDiffProvider { +nonisolated enum InlineDiffProvider { // MARK: - Hunk parsing From e8b27d964f4e267bd811d99efd5708f684c703e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=B5=D0=B4=D0=BE=D1=80=20=D0=91=D0=B0=D1=82=D0=BE?= =?UTF-8?q?=D0=BD=D0=BE=D0=B3=D0=BE=D0=B2?= Date: Tue, 31 Mar 2026 12:34:44 +0300 Subject: [PATCH 3/3] fix: revert nonisolated on InlineDiffProvider to fix build --- Pine/InlineDiffProvider.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pine/InlineDiffProvider.swift b/Pine/InlineDiffProvider.swift index 8c560a2..dbf0302 100644 --- a/Pine/InlineDiffProvider.swift +++ b/Pine/InlineDiffProvider.swift @@ -93,7 +93,7 @@ struct DeletedLinesBlock: Equatable, Sendable { // MARK: - InlineDiffProvider /// Provides diff hunk parsing and accept/revert operations for editor files. -nonisolated enum InlineDiffProvider { +enum InlineDiffProvider { // MARK: - Hunk parsing