Skip to content

fix: make launch at login toggle register app #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Peyton-Spencer merged 3 commits into from
Jul 19, 2026
Merged

fix: make launch at login toggle register app #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ab7d18c
fix: make launch at login toggle register app
Jul 18, 2026
d93da2c
fix: satisfy Swift 6 container isolation checks
Jul 18, 2026
a96c807
fix: reconcile rejected login item changes
Jul 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions Sources/Services/ContainerIsolationService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Foundation

#if canImport(Containerization)
import Containerization
@preconcurrency import Containerization
#endif

/// Service for managing container-based isolation of GitHub Actions runners.
Expand All @@ -21,6 +21,7 @@ import Containerization
/// - Mounted workspace directory
/// - GitHub Actions runner environment
@available(macOS 26.0, *)
@MainActor
class ContainerIsolationService {
#if canImport(Containerization)
// MARK: - Properties
Expand Down Expand Up @@ -103,12 +104,7 @@ class ContainerIsolationService {
// Determine which container image to use
let imageReference = config.containerImage ?? ContainerRunnerConfiguration.defaultRunnerImage

// Create container with specified configuration
let container = try await manager.create(
id,
reference: imageReference,
rootfsSizeInBytes: config.diskSizeInBytes
) { containerConfig in
let configuration: @Sendable (inout LinuxContainer.Configuration) -> Void = { containerConfig in
// Resource allocation
containerConfig.cpus = config.cpuCount
containerConfig.memoryInBytes = config.memoryInBytes
Expand Down Expand Up @@ -146,13 +142,19 @@ class ContainerIsolationService {
// Set environment variables
containerConfig.process.environmentVariables.append("RUNNER_ALLOW_RUNASROOT=1")

// Enable nested virtualization if requested
if config.enableNestedVirtualization {
// Note: This may not be supported in all versions of the framework
// containerConfig.enableNestedVirtualization = true
// Reserved for framework support.
}
}

// Create container with specified configuration
let container = try await manager.create(
id,
reference: imageReference,
rootfsSizeInBytes: config.diskSizeInBytes,
configuration: configuration
)

Comment on lines +151 to +157

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

Reentrancy hazard causing a lost update on containerManager.

Because ContainerIsolationService is a reentrant @MainActor class, reading containerManager into a local manager variable before an await and reassigning self.containerManager = manager afterward (on line 159) creates a Time-Of-Check to Time-Of-Use (TOCTOU) race condition.

If createRunnerContainer is called concurrently (e.g., when launching multiple runners), the concurrent tasks will capture the same initial containerManager state. When they resume from suspension, the later task will blindly overwrite self.containerManager, destroying the internal state (such as tracked containers or resource allocations) mutated by the earlier task.

Consider ensuring that calls to createRunnerContainer are strictly serialized to prevent reentrancy (e.g., using an asynchronous task queue or an isCreating guard), or use a reference-type manager if the Containerization framework supports it.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/Services/ContainerIsolationService.swift` around lines 151 - 157,
Serialize concurrent createRunnerContainer operations so reentrant calls cannot
capture and later overwrite stale containerManager state. Update the container
creation flow around manager and the awaited manager.create call, using an
asynchronous queue or equivalent guard, and preserve all manager mutations from
earlier creations.

// Store mutated manager back
self.containerManager = manager

Expand Down
65 changes: 45 additions & 20 deletions Sources/Services/RunnerManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import Containerization

@MainActor
class RunnerManager: ObservableObject {
enum LoginItemAction: Equatable {
case register
case unregister
case none
}

private enum UpdateStatusMessages {
static let defaultAutomaticChecks = "Checks GitHub releases on launch and once per day."
static let alreadyChecking = "Update check already in progress."
Expand Down Expand Up @@ -96,7 +102,7 @@ class RunnerManager: ObservableObject {
}

reconcileRunnerStates()
syncLoginItem()
reconcileLoginItemSetting()
startStatusPolling()
}

Expand Down Expand Up @@ -352,32 +358,51 @@ class RunnerManager: ObservableObject {

// MARK: - Login Item

/// Synchronize the macOS login item registration with current settings.
/// Reconcile the persisted setting with the current macOS login item state.
///
/// First reconciles the config with the actual OS state (in case the user toggled
/// the login item via System Settings), then registers or unregisters as needed.
/// This is intentionally only performed during initialization. A user change in
/// Mac Runner must be applied to macOS before the system state can be trusted;
/// otherwise enabling the toggle is immediately overwritten by the old state.
private func reconcileLoginItemSetting() {
let osEnabled = SMAppService.mainApp.status == .enabled
guard currentSettings.startOnLogin != osEnabled else { return }

currentSettings.startOnLogin = osEnabled
saveConfiguration()
}

/// Apply the current setting to the macOS login item registration.
private func syncLoginItem() {
let service = SMAppService.mainApp
let osEnabled = service.status == .enabled

// Reconcile: if OS state disagrees with config, trust the OS
if currentSettings.startOnLogin != osEnabled {
currentSettings.startOnLogin = osEnabled
saveConfiguration()
}

do {
if currentSettings.startOnLogin {
if service.status != .enabled {
try service.register()
}
} else {
if service.status == .enabled {
try service.unregister()
}
switch Self.loginItemAction(
startOnLogin: currentSettings.startOnLogin,
isRegistered: service.status == .enabled
) {
case .register:
try service.register()
case .unregister:
try service.unregister()
case .none:
break
}
} catch {
self.error = "Failed to update login item: \(error.localizedDescription)"
let loginItemError = "Failed to update login item: \(error.localizedDescription)"
currentSettings.startOnLogin = service.status == .enabled
saveConfiguration()
self.error = loginItemError
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

nonisolated static func loginItemAction(
startOnLogin: Bool,
isRegistered: Bool
) -> LoginItemAction {
switch (startOnLogin, isRegistered) {
case (true, false): return .register
case (false, true): return .unregister
default: return .none
}
}

Expand Down
14 changes: 14 additions & 0 deletions Tests/MacRunnerTests/MacRunnerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@ final class MacRunnerTests: XCTestCase {
XCTAssertTrue(true)
}

func testEnablingLaunchAtLoginRegistersWhenSystemItemIsDisabled() {
XCTAssertEqual(
RunnerManager.loginItemAction(startOnLogin: true, isRegistered: false),
.register
)
}

func testDisablingLaunchAtLoginUnregistersWhenSystemItemIsEnabled() {
XCTAssertEqual(
RunnerManager.loginItemAction(startOnLogin: false, isRegistered: true),
.unregister
)
}

func testPreferredKernelPathPrefersBundledKernel() {
let bundleURL = URL(fileURLWithPath: "/Applications/MacRunner.app/Contents/Resources")
let appSupportURL = URL(fileURLWithPath: "/Users/test/Library/Application Support/MacRunner")
Expand Down
8 changes: 7 additions & 1 deletion Tests/MacRunnerTests/UpdateCheckerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,13 @@ final class UpdateCheckerTests: XCTestCase {

func testHomebrewInstallDetectionUsesCellarPaths() {
XCTAssertEqual(UpdateChecker.installSource(for: "/opt/homebrew/Cellar/mac-runner/1.2.3/Mac Runner.app"), .homebrewFormula)
XCTAssertEqual(UpdateChecker.installSource(for: "/Applications/Mac Runner.app"), .directDownload)
XCTAssertEqual(
UpdateChecker.installSource(
for: "/Applications/Mac Runner.app",
fileExists: { _ in false }
),
.directDownload
)
}

func testHomebrewInstallDetectionUsesCaskReceipt() {
Expand Down
Loading