feat: add a macos menu bar for sparkdock updates

[paolomainardi]

PR Type

Enhancement

---

Description

• Add Swift menu bar app for update notifications

• Replace launchd service with modern menu bar manager

• Integrate Cirrus CI for end-to-end testing

• Add VS Code development tools and prompts

---

Changes walkthrough 📝

	Relevant files
Enhancement	6 files main.swift Complete Swift menu bar app implementation                              +511/-0  utils.sh Remove old launchd service installation function                  +2/-14    base.yml Add menu bar app build and installation                                    +69/-0    install.macos Remove launchd service installation from installer              +8/-12    sparkdock.macos Disable notifications and remove launchd calls                      +5/-3      Justfile Add Tart VM and Cirrus CI testing commands                              +63/-0
Tests	3 files .cirrus.yml Add Cirrus CI end-to-end testing configuration                      +32/-0    test-menubar-app.yml Add GitHub Actions workflow for Swift app                                +93/-0    SparkdockManagerTests.swift Unit tests for Swift menu bar app                                                +54/-0
Configuration changes	8 files Package.swift Swift Package Manager configuration for menu app                  +26/-0    Makefile Build and installation commands for menu app                          +44/-0    com.sparkfabrik.sparkdock.menubar.plist LaunchAgent configuration for menu bar app                              +20/-0    menu.json Menu structure configuration for dynamic items                      +37/-0    devcontainer.json Add Swift extension and MCP server configuration                  +15/-2    test-ansible-playbook.yml Limit testing to macOS 15 and latest                                          +1/-2      test-sjust.yml Simplify to single macOS 15 runner                                              +1/-5      Info.plist Bundle configuration for menu bar app                                        +34/-0
Documentation	6 files copilot-instructions.md Comprehensive development guide and coding standards          +142/-7  swift-macos-engineer.md Claude agent for Swift macOS development                                  +36/-0    swift-macos-engineer.prompt.md VS Code prompt for Swift development assistance                    +74/-0    CLAUDE.md Claude Code integration guide for repository                          +133/-0  sparkdock-manager.md Technical specification for menu bar app                                  +282/-0  README.md Menu bar app usage and installation guide                                +67/-0
Dependencies	1 files all-packages.yml Update Node.js from version 20 to 24                                          +2/-2
Miscellaneous	1 files .gitkeep Placeholder for logo resources directory                                  +2/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[sparkfabrik-ai-bot]

PR Reviewer Guide 🔍

(Review updated until commit 1877761)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 Security concerns Command injection: The Swift menu bar app executes terminal commands through AppleScript with basic string escaping. While the current implementation replaces quotes and backslashes, this approach could be vulnerable to command injection if malicious data reaches the executeTerminalCommand function. Consider using more robust command sanitization or parameterized execution methods.

⚡ Recommended focus areas for review Resource Loading The resource loading logic has complex fallback paths between Bundle.module and Bundle.main with extensive debug logging. The error handling for missing logo resources includes detailed directory listing which could expose system information in logs. if cachedLogoImage == nil { var foundLogo = false if let path = Bundle.module.path(forResource: AppConstants.logoResourceName, ofType: "png") { cachedLogoImage = NSImage(contentsOfFile: path) foundLogo = true } else if let path = Bundle.main.path(forResource: AppConstants.logoResourceName, ofType: "png") { cachedLogoImage = NSImage(contentsOfFile: path) foundLogo = true } if !foundLogo { let moduleResourcePath = Bundle.module.resourcePath ?? "<nil>" let mainResourcePath = Bundle.main.resourcePath ?? "<nil>" let modulePngs = (try? FileManager.default.contentsOfDirectory(atPath: moduleResourcePath).filter { $0.hasSuffix(".png") }) ?? [] let mainPngs = (try? FileManager.default.contentsOfDirectory(atPath: mainResourcePath).filter { $0.hasSuffix(".png") }) ?? [] let modulePngList = modulePngs.joined(separator: ", ") let mainPngList = mainPngs.joined(separator: ", ") AppConstants.logger.error("Logo resource '\(AppConstants.logoResourceName).png' not found. Checked paths: module=\(moduleResourcePath), main=\(mainResourcePath). Available PNGs in module: \(modulePngList). Available PNGs in main: \(mainPngList).") } } File Operation The rsync command replaces cp -R for copying files in CI environments. This change should be verified to ensure all necessary files are copied correctly and permissions are preserved, especially for executable scripts. rsync -azh --exclude='.git' . /opt/sparkdock Conditional Logic The LaunchAgent installation has complex conditional logic that skips installation in CI environments. The condition checks multiple environment variables and could potentially fail to detect some CI environments or incorrectly skip installation in valid scenarios. when: not (ansible_env.CI is defined or ansible_env.GITHUB_ACTIONS is defined or ansible_env.RUNNER_OS is defined or ansible_env.HOME == '/var/root')

[paolomainardi]

/review

[paolomainardi]

/improve

[sparkfabrik-ai-bot]

Persistent review updated to latest commit aa137df

[paolomainardi]

/review

[paolomainardi]

/describe

[paolomainardi]

/improve

[sparkfabrik-ai-bot]

Persistent review updated to latest commit dad4d5b

[sparkfabrik-ai-bot]

PR Description updated to latest commit (dad4d5b)

[paolomainardi]

/review

[paolomainardi]

/improve

[sparkfabrik-ai-bot]

Persistent review updated to latest commit de90609

[sparkfabrik-ai-bot]

PR Description updated to latest commit (de90609)

[paolomainardi]

/describe

[paolomainardi]

/review

[paolomainardi]

/improve

[sparkfabrik-ai-bot]

PR Code Suggestions ✨

Latest suggestions up to 1877761

Category	Suggestion	Impact
Security	Add executable file validation The process execution lacks proper security validation and could be vulnerable to path traversal or execution of unintended binaries. Validate the executable's signature or checksum before execution. src/menubar-app/Sources/SparkdockManager/main.swift [274-283] private func runSparkdockCheck() async -> Bool { - guard FileManager.default.fileExists(atPath: AppConstants.sparkdockExecutablePath) else { - AppConstants.logger.warning("Sparkdock executable not found at \(AppConstants.sparkdockExecutablePath)") + let executablePath = AppConstants.sparkdockExecutablePath + guard FileManager.default.fileExists(atPath: executablePath) else { + AppConstants.logger.warning("Sparkdock executable not found at \(executablePath)") + return false + } + + // Verify the executable is actually executable and not a directory + guard FileManager.default.isExecutableFile(atPath: executablePath) else { + AppConstants.logger.error("File at \(executablePath) is not executable") return false } let process = Process() - process.executableURL = URL(fileURLWithPath: AppConstants.sparkdockExecutablePath) + process.executableURL = URL(fileURLWithPath: executablePath) process.arguments = ["check-updates"] Suggestion importance[1-10]: 7 __ Why: Adding isExecutableFile validation is a good security practice that prevents attempting to execute non-executable files, though the hardcoded path /opt/sparkdock/bin/sparkdock.macos limits the attack surface.	Medium
	Remove sensitive environment variable logging Debug tasks that output sensitive environment information should be conditional or removed in production. This task exposes system details that could be security-sensitive in logs. ansible/macos/macos/base.yml [408-414] - name: Check CI environment debug: msg: - - "HOME: {{ ansible_env.HOME | default('undefined') }}" - - "USER: {{ ansible_env.USER | default('undefined') }}" - "CI: {{ ansible_env.CI | default('undefined') }}" - "GITHUB_ACTIONS: {{ ansible_env.GITHUB_ACTIONS | default('undefined') }}" - "RUNNER_OS: {{ ansible_env.RUNNER_OS | default('undefined') }}" + when: ansible_env.CI is defined or ansible_env.GITHUB_ACTIONS is defined Suggestion importance[1-10]: 4 __ Why: While removing HOME and USER from debug output is a minor security improvement, these are standard environment variables and the debug task is already conditional for CI environments.	Low
General	Increase VM memory allocation The memory allocation of 4GB may be insufficient for macOS Sequoia with Sparkdock installation and testing. Consider increasing to 8GB to prevent out-of-memory issues during Ansible provisioning and Swift compilation. .cirrus.yml [1-7] task: name: "Sparkdock End-to-End Test" # Use Tart VM for testing macos_instance: image: ghcr.io/cirruslabs/macos-sequoia-base:latest cpu: 2 - memory: 4 + memory: 8 Suggestion importance[1-10]: 6 __ Why: Increasing memory from 4GB to 8GB is a reasonable optimization for macOS Sequoia VMs running Ansible and Swift compilation, though the current allocation may still work.	Low
	Add symlink validation before removal The hardcoded path removal is fragile and may fail on different system configurations. Add error handling and verify the symlink target before removal to prevent accidental deletion of valid files. bin/install.macos [123-127] # Try to remove spurious links, they are part of the tart VM. # Yarn: /opt/homebrew/lib/node_modules/yarn/bin/yarn.js if [[ -L /opt/homebrew/lib/node_modules/yarn/bin/yarn.js ]]; then - sudo rm -f /opt/homebrew/lib/node_modules/yarn/bin/yarn.js + if ! readlink /opt/homebrew/lib/node_modules/yarn/bin/yarn.js >/dev/null 2>&1; then + sudo rm -f /opt/homebrew/lib/node_modules/yarn/bin/yarn.js || true + fi fi Suggestion importance[1-10]: 5 __ Why: The suggestion adds error handling with || true and symlink validation, but the improvement is minor since the original code already checks if it's a symlink with -L.	Low

---

Previous suggestions

✅ Suggestions up to commit de90609

Category	Suggestion	Impact
General	Add SSH retry mechanism with timeout The SSH connection attempt should include a timeout and retry mechanism since VMs may take longer to become SSH-ready even after getting an IP address. Justfile [37-45] VM_IP=$(tart ip sparkdock-test 2>/dev/null || echo "") if [ -n "$VM_IP" ]; then echo "Connecting to VM at $VM_IP..." - ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null admin@$VM_IP + for i in {1..5}; do + if ssh -o ConnectTimeout=10 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null admin@$VM_IP exit 2>/dev/null; then + ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null admin@$VM_IP + break + else + echo "SSH attempt $i failed, retrying in 10 seconds..." + sleep 10 + fi + done else echo "❌ Could not get VM IP address. Make sure the VM is running." echo "You can manually check with: tart list" exit 1 fi Suggestion importance[1-10]: 8 __ Why: The SSH retry mechanism addresses a real timing issue where VMs may have an IP but aren't ready for SSH connections yet, significantly improving the reliability of the automated testing workflow.	Medium
	Add error handling for VM creation The VM creation process should handle potential failures during cloning. Add error checking to ensure the clone operation succeeds before reporting success. Justfile [22-25] if ! tart list | grep -q "sparkdock-test"; then - tart clone {{IMAGE}} sparkdock-test - echo "✅ VM 'sparkdock-test' created successfully" + if tart clone {{IMAGE}} sparkdock-test; then + echo "✅ VM 'sparkdock-test' created successfully" + else + echo "❌ Failed to create VM 'sparkdock-test'" + exit 1 + fi fi Suggestion importance[1-10]: 7 __ Why: Adding proper error handling for the tart clone command improves reliability and provides better feedback when VM creation fails, which is important for CI/CD workflows.	Medium
	Increase VM memory allocation The memory allocation of 4GB may be insufficient for macOS Sequoia with development tools. Consider increasing to 8GB to prevent out-of-memory issues during Sparkdock installation and testing. .cirrus.yml [1-7] task: name: "Sparkdock End-to-End Test" # Use Tart VM for testing macos_instance: image: ghcr.io/cirruslabs/macos-sequoia-base:latest cpu: 2 - memory: 4 + memory: 8 Suggestion importance[1-10]: 6 __ Why: Increasing memory from 4GB to 8GB is a reasonable improvement for macOS Sequoia development environments, but the current allocation may still be sufficient for basic testing.	Low

Suggestions up to commit eabfa65

Category	Suggestion	Impact
Possible issue	Fix process termination handler race condition The terminationHandler is set after process.run() which creates a race condition. If the process completes very quickly, the handler may not be set in time to capture the termination status. Set the termination handler before starting the process to ensure it's always captured. src/menubar-app/Sources/SparkdockManager/main.swift [274-318] private func runSparkdockCheck() async -> Bool { guard FileManager.default.fileExists(atPath: AppConstants.sparkdockExecutablePath) else { AppConstants.logger.warning("Sparkdock executable not found at \(AppConstants.sparkdockExecutablePath)") return false } let process = Process() process.executableURL = URL(fileURLWithPath: AppConstants.sparkdockExecutablePath) process.arguments = ["check-updates"] var terminationStatus: Int32 = -1 do { - try process.run() - - // Await process termination with timeout + // Set termination handler BEFORE running the process let finished: Bool = await withTaskCancellationHandler( operation: { do { terminationStatus = try await withTimeout(seconds: AppConstants.processTimeout) { await withCheckedContinuation { continuation in process.terminationHandler = { proc in continuation.resume(returning: proc.terminationStatus) } + try? process.run() } } return true } catch { return false } }, onCancel: { // If cancelled, terminate the process process.terminate() } ) Suggestion importance[1-10]: 8 __ Why: This addresses a critical race condition where the terminationHandler is set after process.run(), potentially missing fast-completing processes. The fix ensures the handler is always set before execution.	Medium
Security	Improve command escaping for security The command escaping only handles double quotes but not other shell metacharacters like backslashes, single quotes, or special characters. This could lead to command injection vulnerabilities if malicious input is processed. Use proper shell escaping or validate input more thoroughly. src/menubar-app/Sources/SparkdockManager/main.swift [349-372] private func executeTerminalCommand(_ command: String) { let process = Process() // Use osascript to run AppleScript more securely process.executableURL = URL(fileURLWithPath: "/usr/bin/osascript") + + // Properly escape the command for AppleScript + let escapedCommand = command + .replacingOccurrences(of: "\\", with: "\\\\") + .replacingOccurrences(of: "\"", with: "\\\"") + .replacingOccurrences(of: "\n", with: "\\n") + .replacingOccurrences(of: "\r", with: "\\r") + // Create AppleScript to open Terminal and run command let appleScript = """ tell application "Terminal" activate if (count of windows) > 0 then - do script "\(command.replacingOccurrences(of: "\"", with: "\\\""))" in front window + do script "\(escapedCommand)" in front window else - do script "\(command.replacingOccurrences(of: "\"", with: "\\\""))" + do script "\(escapedCommand)" end if end tell """ process.arguments = ["-e", appleScript] do { try process.run() AppConstants.logger.info("Executed terminal command: \(command)") } catch { AppConstants.logger.error("Failed to execute terminal command '\(command)': \(error.localizedDescription)") showErrorAlert("Command Execution Error", "Failed to execute command: \(command)") } } Suggestion importance[1-10]: 6 __ Why: The suggestion improves security by adding more comprehensive escaping for AppleScript strings, though the current implementation already provides reasonable protection since commands come from a controlled JSON configuration.	Low

Suggestions up to commit dad4d5b

Category	Suggestion	Impact
Security	Fix AppleScript injection vulnerability The command escaping is incomplete and could lead to AppleScript injection vulnerabilities. Single quotes and other special characters need proper escaping to prevent script execution attacks. src/menubar-app/Sources/SparkdockMenubar/main.swift [192-205] private func executeTerminalCommand(_ command: String) { - let escapedCommand = command.replacingOccurrences(of: "\\", with: "\\\\") + // Comprehensive escaping for AppleScript safety + let escapedCommand = command + .replacingOccurrences(of: "\\", with: "\\\\") .replacingOccurrences(of: "\"", with: "\\\"") + .replacingOccurrences(of: "'", with: "\\'") + .replacingOccurrences(of: "\n", with: "\\n") + .replacingOccurrences(of: "\r", with: "\\r") + .replacingOccurrences(of: "\t", with: "\\t") // Try iTerm first, fallback to Terminal let iTermScript = """ tell application "iTerm" activate create window with default profile tell current session of current window write text "\(escapedCommand)" end tell end tell """ Suggestion importance[1-10]: 8 __ Why: Important security improvement. The current escaping only handles backslashes and quotes, but the suggestion correctly identifies that other special characters like newlines and tabs could be exploited for AppleScript injection attacks.	Medium
General	Add process timeout handling Add timeout handling to prevent the process from hanging indefinitely. Long-running Git operations could cause the app to become unresponsive during update checks. src/menubar-app/Sources/SparkdockMenubar/main.swift [137-150] private func runSparkdockCheck() -> Bool { let process = Process() process.executableURL = URL(fileURLWithPath: AppConstants.sparkdockExecutablePath) process.arguments = ["check-updates"] do { try process.run() - process.waitUntilExit() + + // Add timeout to prevent hanging + let timeout = DispatchTime.now() + .seconds(30) + let semaphore = DispatchSemaphore(value: 0) + + DispatchQueue.global().async { + process.waitUntilExit() + semaphore.signal() + } + + if semaphore.wait(timeout: timeout) == .timedOut { + process.terminate() + print("Sparkdock check timed out after 30 seconds") + return false + } + return process.terminationStatus == 0 } catch { print("Failed to run sparkdock check: \(error)") return false } } Suggestion importance[1-10]: 7 __ Why: Good suggestion to prevent hanging processes during Git operations. The timeout mechanism using DispatchSemaphore is properly implemented and addresses a real issue where network or Git operations could cause the app to become unresponsive.	Medium
	Handle missing executable gracefully The app continues running even when the Sparkdock executable is missing, which will cause all update operations to fail silently. Consider showing a user-visible error or disabling functionality when the executable is not found. src/menubar-app/Sources/SparkdockMenubar/main.swift [29-37] func applicationDidFinishLaunching(_ notification: Notification) { - if !FileManager.default.fileExists(atPath: AppConstants.sparkdockExecutablePath) { + let sparkdockExists = FileManager.default.fileExists(atPath: AppConstants.sparkdockExecutablePath) + + if !sparkdockExists { print("Warning: Sparkdock executable not found at \(AppConstants.sparkdockExecutablePath)") + // Show user notification about missing executable + let alert = NSAlert() + alert.messageText = "Sparkdock Not Found" + alert.informativeText = "The Sparkdock executable was not found. Update checking will be disabled." + alert.alertStyle = .warning + alert.runModal() } setupMenuBar() - setupUpdateTimer() - checkForUpdates() + if sparkdockExists { + setupUpdateTimer() + checkForUpdates() + } } Suggestion importance[1-10]: 6 __ Why: Reasonable improvement for user experience. Showing an alert when the sparkdock executable is missing provides better feedback than silent failures, though the modal alert approach could be intrusive.	Low
	Add app state awareness The timer is not invalidated when the app becomes inactive or when system resources are constrained. This could lead to unnecessary background processing and battery drain. src/menubar-app/Sources/SparkdockMenubar/main.swift [114-120] private func setupUpdateTimer() { updateTimer = Timer.scheduledTimer(withTimeInterval: AppConstants.updateInterval, repeats: true) { [weak self] _ in self?.checkForUpdates() } updateTimer?.tolerance = 60.0 + + // Observe app state changes to pause/resume timer + NotificationCenter.default.addObserver( + forName: NSApplication.didBecomeActiveNotification, + object: nil, + queue: .main + ) { [weak self] _ in + self?.resumeUpdateTimer() + } + + NotificationCenter.default.addObserver( + forName: NSApplication.didResignActiveNotification, + object: nil, + queue: .main + ) { [weak self] _ in + self?.pauseUpdateTimer() + } } +private func pauseUpdateTimer() { + updateTimer?.invalidate() +} + +private func resumeUpdateTimer() { + setupUpdateTimer() +} + Suggestion importance[1-10]: 5 __ Why: The suggestion adds complexity for minimal benefit. Menu bar apps typically run continuously and the 4-hour update interval is already conservative. The added notification observers and pause/resume logic may not provide significant battery savings.	Low

✅ Suggestions up to commit aa137df

Category	Suggestion	Impact
Possible issue	Add fallback icon when logo missing The function returns nil when no logo is found, but the menu bar app should always display an icon. Add a fallback to generate a default icon when the logo resource is missing to ensure the app remains functional. src/menubar-app/Sources/SparkdockMenubar/main.swift [221-231] private func loadIcon(hasUpdates: Bool) -> NSImage? { // Try to load the logo var logoImage: NSImage? if let path = Bundle.main.path(forResource: AppConstants.logoResourceName, ofType: "png") { logoImage = NSImage(contentsOfFile: path) } else if let path = Bundle.module.path(forResource: AppConstants.logoResourceName, ofType: "png") { logoImage = NSImage(contentsOfFile: path) } - guard let logo = logoImage else { return nil } + // Fallback to system gear icon if logo not found + let baseImage = logoImage ?? NSImage(systemSymbolName: "gear", accessibilityDescription: "Sparkdock") + guard let logo = baseImage else { return nil } ... Suggestion importance[1-10]: 7 __ Why: Good suggestion to add fallback icon when logo is missing. The current code returns nil which could cause the menu bar app to have no icon, but the suggested fallback using NSImage(systemSymbolName:) ensures functionality.	Medium
General	Fix failing test for optional logo This test will fail in CI environments where the logo doesn't exist, causing build failures. Make the test conditional or remove the hard assertion to prevent CI pipeline issues. src/menubar-app/Tests/SparkdockMenubarTests/SparkdockMenubarTests.swift [11-17] func testResourcesExist() { // Test that the logo resource file exists in the source tree let currentDir = FileManager.default.currentDirectoryPath let logoPath = "\(currentDir)/Sources/SparkdockMenubar/Resources/sparkfabrik-logo.png" let fileExists = FileManager.default.fileExists(atPath: logoPath) - XCTAssertTrue(fileExists, "SparkFabrik logo should exist at: \(logoPath)") + + // Don't fail the test if logo is missing - it's optional + if fileExists { + print("✅ SparkFabrik logo found at: \(logoPath)") + } else { + print("ℹ️ SparkFabrik logo not found (optional): \(logoPath)") + } } Suggestion importance[1-10]: 6 __ Why: Valid suggestion to prevent CI failures. The test currently uses XCTAssertTrue(fileExists) which will fail when the optional logo is missing, and the improved code makes it informational rather than failing.	Low

[sparkfabrik-ai-bot]

Persistent review updated to latest commit 1877761

[sparkfabrik-ai-bot]

PR Description updated to latest commit (1877761)

[Copilot]

Pull Request Overview

This PR adds a native Swift macOS menu bar application to replace the previous launchd-based update notification system. The new SparkdockManager provides visual update indicators and integrates seamlessly with the Sparkdock development environment.

• Complete Swift menu bar app implementation with modern async/await patterns
• Removal of old launchd service in favor of lightweight event-driven notifications
• Enhanced CI/CD pipeline with Cirrus CLI integration and comprehensive testing

Reviewed Changes

Copilot reviewed 22 out of 27 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
main.swift	Complete Swift menu bar app with update monitoring and configurable menu system
base.yml	Ansible integration for automated app building and installation
utils.sh	Removal of legacy launchd service installation function
install.macos	Updated installer without launchd dependency
sparkdock.macos	Disabled legacy notifications in favor of menu bar app
Package.swift	Swift Package Manager configuration for the menu bar app
Makefile	Build automation and installation commands
test-menubar-app.yml	GitHub Actions workflow for Swift app testing
Justfile	Added Tart VM and Cirrus CI testing infrastructure

---

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

[paolomainardi]

closes #129