Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import Foundation

public enum MobileMacPowerError: Error, Equatable, Sendable {
case unavailable
case unsupported
case invalidResponse
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import Foundation

public struct MobileMacPowerStatus: Equatable, Sendable {
public let keepAwakeEnabled: Bool
public let lowPowerEnabled: Bool

static func decode(_ data: Data) throws -> MobileMacPowerStatus {
let envelope = try JSONSerialization.jsonObject(with: data) as? [String: Any]
guard envelope?["ok"] as? Bool == true,
let result = envelope?["result"] as? [String: Any],
let keepAwake = result["keep_awake_enabled"] as? Bool,
let lowPower = result["low_power_enabled"] as? Bool else {
throw MobileMacPowerError.invalidResponse
}
return MobileMacPowerStatus(
keepAwakeEnabled: keepAwake,
lowPowerEnabled: lowPower
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import CmuxMobileRPC
import Foundation

extension MobileShellComposite {
public var supportsMacPowerControl: Bool {
supportedHostCapabilities.contains(Self.macPowerCapability)
}

public func macPowerStatus() async throws -> MobileMacPowerStatus {
try await sendMacPowerRequest(method: "mac.power.status", params: [:])
}

public func sleepMacDisplay() async throws {
guard supportsMacPowerControl, let remoteClient else { throw MobileMacPowerError.unavailable }
let request = try MobileCoreRPCClient.requestData(method: "mac.power.sleep_display", params: [:])
_ = try await remoteClient.sendRequest(request)
}

public func setMacKeepAwake(_ enabled: Bool) async throws -> MobileMacPowerStatus {
try await sendMacPowerRequest(
method: "mac.power.keep_awake.set", params: ["enabled": enabled]
)
}

public func setMacLowPowerMode(_ enabled: Bool) async throws -> MobileMacPowerStatus {
try await sendMacPowerRequest(
method: "mac.power.low_power.set", params: ["enabled": enabled]
)
}

private func sendMacPowerRequest(
method: String, params: [String: Any]
) async throws -> MobileMacPowerStatus {
guard supportsMacPowerControl, let remoteClient else { throw MobileMacPowerError.unavailable }
let request = try MobileCoreRPCClient.requestData(method: method, params: params)
let response = try await remoteClient.sendRequest(request)
return try MobileMacPowerStatus.decode(response)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking {
static let terminalArtifactCapability = "terminal.artifact.v1"
static let dogfoodFeedbackCapability = "dogfood.v1"
static let workspaceGroupsCapability = "workspace.groups.v1"
static let macPowerCapability = "mac.power.control.v1"
private static let terminalOutputCapabilityTimeoutNanoseconds: UInt64 = 750_000_000
/// How long the render-grid stream may stay silent (no event of any topic)
/// before the liveness watchdog suspects the push subscription is dead and
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import Foundation
import Testing
@testable import CmuxMobileShell

@Suite struct MobileMacPowerStatusTests {
@Test func decodesAuthoritativeStatus() throws {
let data = try JSONSerialization.data(withJSONObject: [
"ok": true,
"result": [
"keep_awake_enabled": true,
"low_power_enabled": false,
],
])
let status = try MobileMacPowerStatus.decode(data)
#expect(status.keepAwakeEnabled)
#expect(!status.lowPowerEnabled)
}

@Test func rejectsIncompleteStatus() throws {
let data = try JSONSerialization.data(withJSONObject: [
"ok": true,
"result": ["keep_awake_enabled": true],
])
#expect(throws: MobileMacPowerError.invalidResponse) {
try MobileMacPowerStatus.decode(data)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ struct MacComputerDetailView: View {
@State private var pendingCustomName: String?
@State private var pendingCustomColor: String?
@State private var pendingCustomIcon: String?
@State private var macPowerStatus: MobileMacPowerStatus?
@State private var macPowerError: String?
@State private var isMacPowerBusy = false
@State private var pendingDisplaySleep = false

/// Curated icon choices: a few computer/utility SF Symbols + emojis.
private static let symbolChoices = [
Expand Down Expand Up @@ -70,6 +74,7 @@ struct MacComputerDetailView: View {
presenceSection
routesSection
identitySection
if isForeground && store.supportsMacPowerControl { macPowerSection }
actionsSection
}
.navigationTitle(displayTitle)
Expand All @@ -86,6 +91,18 @@ struct MacComputerDetailView: View {
customColorPick = color
}
}
.confirmationDialog(
L10n.string("mobile.macPower.sleep.confirm.title", defaultValue: "Sleep Mac display?"),
isPresented: $pendingDisplaySleep,
titleVisibility: .visible
) {
Button(L10n.string("mobile.macPower.sleep", defaultValue: "Sleep Display"), role: .destructive) {
runPowerAction { try await store.sleepMacDisplay() }
}
Button(L10n.string("mobile.common.cancel", defaultValue: "Cancel"), role: .cancel) {}
} message: {
Text(L10n.string("mobile.macPower.sleep.confirm.message", defaultValue: "The Mac stays awake, but all connected displays turn off."))
}
.confirmationDialog(
"\(L10n.string("mobile.computers.removeTitlePrefix", defaultValue: "Remove")) \(displayTitle)?",
isPresented: $pendingRemoval,
Expand All @@ -100,6 +117,10 @@ struct MacComputerDetailView: View {
} message: {
Text(removeMessage)
}
.task(id: "\(isForeground)-\(store.supportsMacPowerControl)") {
guard isForeground, store.supportsMacPowerControl else { return }
await refreshMacPower()
}
Comment on lines +120 to +123

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Prefer an array for the .task ID to avoid string allocations.

Using string interpolation for the .task identifier allocates a new string on every render pass of this view. Since both properties are booleans, you can pass them as an Equatable array to achieve the exact same invalidation semantics with less overhead.

💡 Proposed refactor
-        .task(id: "\(isForeground)-\(store.supportsMacPowerControl)") {
+        .task(id: [isForeground, store.supportsMacPowerControl]) {
             guard isForeground, store.supportsMacPowerControl else { return }
             await refreshMacPower()
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.task(id: "\(isForeground)-\(store.supportsMacPowerControl)") {
guard isForeground, store.supportsMacPowerControl else { return }
await refreshMacPower()
}
.task(id: [isForeground, store.supportsMacPowerControl]) {
guard isForeground, store.supportsMacPowerControl else { return }
await refreshMacPower()
}
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerDetailView.swift`
around lines 120 - 123, Update the .task modifier in MacComputerDetailView to
use an Equatable array containing isForeground and store.supportsMacPowerControl
as its ID instead of interpolating them into a string. Preserve the existing
guard and refreshMacPower behavior and the same invalidation semantics.

}

// MARK: - Appearance editing
Expand Down Expand Up @@ -450,6 +471,91 @@ struct MacComputerDetailView: View {
}
}


@ViewBuilder
private var macPowerSection: some View {
Section(L10n.string("mobile.macPower.section", defaultValue: "Mac Power")) {
Toggle(
L10n.string("mobile.macPower.keepAwake", defaultValue: "Keep Mac Awake"),
isOn: Binding(
get: { macPowerStatus?.keepAwakeEnabled ?? false },
set: { enabled in
runPowerStatusAction { try await store.setMacKeepAwake(enabled) }
}
)
)
.disabled(isMacPowerBusy || macPowerStatus == nil)
.accessibilityIdentifier("MobileMacKeepAwakeSwitch")

Toggle(
L10n.string("mobile.macPower.lowPower", defaultValue: "Low Power Mode"),
isOn: Binding(
get: { macPowerStatus?.lowPowerEnabled ?? false },
set: { enabled in
runPowerStatusAction { try await store.setMacLowPowerMode(enabled) }
}
)
)
.disabled(isMacPowerBusy || macPowerStatus == nil)
.accessibilityIdentifier("MobileMacLowPowerSwitch")

Button {
pendingDisplaySleep = true
} label: {
Label(L10n.string("mobile.macPower.sleep", defaultValue: "Sleep Display"), systemImage: "moon.fill")
}
.disabled(isMacPowerBusy)
.accessibilityIdentifier("MobileMacSleepDisplayButton")

if isMacPowerBusy { ProgressView() }
if let macPowerError {
Text(macPowerError).foregroundStyle(.red)
.accessibilityIdentifier("MobileMacPowerError")
}
}
}

private func refreshMacPower() async {
isMacPowerBusy = true
defer { isMacPowerBusy = false }
do {
let status = try await store.macPowerStatus()
macPowerStatus = status
macPowerError = nil
} catch {
macPowerError = L10n.string("mobile.macPower.error", defaultValue: "Couldn't update Mac power controls.")
}
}

private func runPowerAction(_ action: @escaping @MainActor () async throws -> Void) {
guard !isMacPowerBusy else { return }
isMacPowerBusy = true
macPowerError = nil
Task {
defer { isMacPowerBusy = false }
do { try await action() }
catch { macPowerError = L10n.string("mobile.macPower.error", defaultValue: "Couldn't update Mac power controls.") }
}
}

private func runPowerStatusAction(
_ action: @escaping @MainActor () async throws -> MobileMacPowerStatus
) {
guard !isMacPowerBusy else { return }
isMacPowerBusy = true
macPowerError = nil
Task {
defer { isMacPowerBusy = false }
do {
let status = try await action()
macPowerStatus = status
} catch {
macPowerError = L10n.string("mobile.macPower.error", defaultValue: "Couldn't update Mac power controls.")
await refreshMacPower()
}
}
}

Comment on lines +541 to +558

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Prevent refreshMacPower() from wiping out the mutation error message.

If the backend mutation (e.g., setMacLowPowerMode) fails, the catch block correctly sets macPowerError to notify the user. However, await refreshMacPower() is called immediately afterward. Because refreshMacPower() sets macPowerError = nil upon a successful status fetch, the error message meant for the user is instantly wiped out.

Swap the order so the UI resynchronizes the toggle status before you assign the failure message for the mutation.

🐛 Proposed fix
     private func runPowerStatusAction(
         _ action: `@escaping` `@MainActor` () async throws -> MobileMacPowerStatus
     ) {
         guard !isMacPowerBusy else { return }
         isMacPowerBusy = true
         macPowerError = nil
         Task {
             defer { isMacPowerBusy = false }
             do {
                 let status = try await action()
                 macPowerStatus = status
             } catch {
-                macPowerError = L10n.string("mobile.macPower.error", defaultValue: "Couldn't update Mac power controls.")
                 await refreshMacPower()
+                macPowerError = L10n.string("mobile.macPower.error", defaultValue: "Couldn't update Mac power controls.")
             }
         }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private func runPowerStatusAction(
_ action: @escaping @MainActor () async throws -> MobileMacPowerStatus
) {
guard !isMacPowerBusy else { return }
isMacPowerBusy = true
macPowerError = nil
Task {
defer { isMacPowerBusy = false }
do {
let status = try await action()
macPowerStatus = status
} catch {
macPowerError = L10n.string("mobile.macPower.error", defaultValue: "Couldn't update Mac power controls.")
await refreshMacPower()
}
}
}
private func runPowerStatusAction(
_ action: `@escaping` `@MainActor` () async throws -> MobileMacPowerStatus
) {
guard !isMacPowerBusy else { return }
isMacPowerBusy = true
macPowerError = nil
Task {
defer { isMacPowerBusy = false }
do {
let status = try await action()
macPowerStatus = status
} catch {
await refreshMacPower()
macPowerError = L10n.string("mobile.macPower.error", defaultValue: "Couldn't update Mac power controls.")
}
}
}
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerDetailView.swift`
around lines 541 - 558, Update the catch block in runPowerStatusAction to await
refreshMacPower() before assigning macPowerError, so the status refresh cannot
clear the mutation failure message. Preserve the existing localized error text
and refresh behavior.

private var connectionPhrase: String {
switch connectionStatus {
case .connected: return L10n.string("mobile.deviceTree.connected", defaultValue: "Connected")
Expand Down
49 changes: 49 additions & 0 deletions Sources/MacPowerService.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import Foundation

@MainActor
final class MacPowerService {
enum ServiceError: Error, Equatable { case assertionFailed, lowPowerMutationFailed }

private let powerControls: any SleepyPowerControlling
private let assertionHolder: any PowerAssertionHolding

init(
powerControls: any SleepyPowerControlling,
assertionHolder: any PowerAssertionHolding
) {
self.powerControls = powerControls
self.assertionHolder = assertionHolder
}

convenience init() {
self.init(
powerControls: SleepyPowerControls(),
assertionHolder: PowerAssertionHolder()
)
}

func status() async -> MacPowerStatus {
MacPowerStatus(
keepAwakeEnabled: assertionHolder.isEnabled,
lowPowerEnabled: await powerControls.isLowPowerOn()
)
}

func sleepDisplay() async { await powerControls.sleepDisplayNow() }

func setKeepAwake(_ enabled: Bool) async throws -> MacPowerStatus {
do { try assertionHolder.setEnabled(enabled) }
catch { throw ServiceError.assertionFailed }
return await status()
}

func setLowPowerMode(_ enabled: Bool) async throws -> MacPowerStatus {
let applied = await powerControls.setLowPowerMode(enabled)
guard applied == enabled else { throw ServiceError.lowPowerMutationFailed }
let authoritative = await status()
guard authoritative.lowPowerEnabled == enabled else {
throw ServiceError.lowPowerMutationFailed
}
return authoritative
}
}
6 changes: 6 additions & 0 deletions Sources/MacPowerStatus.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import Foundation

struct MacPowerStatus: Equatable, Sendable {
let keepAwakeEnabled: Bool
let lowPowerEnabled: Bool
}
1 change: 1 addition & 0 deletions Sources/Mobile/MobileHostService+Capabilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ extension MobileHostService {
"chat.artifact.v1",
"chat.artifact.gallery.v1",
"dogfood.v1",
"mac.power.control.v1",
// The workspace list carries group sections (group_id per workspace +
// a top-level groups array) and the host accepts
// workspace.group.collapse/expand from mobile. iOS feature-detects
Expand Down
3 changes: 3 additions & 0 deletions Sources/Mobile/MobileHostService+TicketAuthorization.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ extension MobileHostService {
)
case "mobile.events.subscribe", "mobile.events.unsubscribe":
return nil
case "mac.power.status", "mac.power.sleep_display",
"mac.power.keep_awake.set", "mac.power.low_power.set":
return ticketMacScopedWorkspaceMutationAuthorizationError(authorization: authorization)
case "mobile.host.status":
return nil
default:
Expand Down
7 changes: 6 additions & 1 deletion Sources/Mobile/MobileHostService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1086,10 +1086,15 @@ final class MobileHostService {
guard requiresAuthorization(method: request.method) else { return nil }
return await stackAuthorization(request)
case .irohAdmission:
return nil
guard isPrivilegedMacPowerMethod(request.method) else { return nil }
return await stackAuthorization(request)
}
}

nonisolated private static func isPrivilegedMacPowerMethod(_ method: String) -> Bool {
method == "mac.power.status" || method.hasPrefix("mac.power.")
}
Comment on lines +1094 to +1096

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Redundant first condition in isPrivilegedMacPowerMethod

method == "mac.power.status" is always true when method.hasPrefix("mac.power.") is true, so the first arm of the || is dead code. A reader may believe the two conditions guard different sets — for example, that the prefix test covers a broader set than just mac.power.status — when they are actually identical in coverage. The function can be simplified to just the hasPrefix guard.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +1094 to +1096

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 hasPrefix("mac.power.") is broader than the explicit ticket-auth list

isPrivilegedMacPowerMethod uses method.hasPrefix("mac.power.") for the .irohAdmission path, so every future mac.power.* method added to the v2MobileMacPower switch automatically requires Stack auth over Iroh. However, the ticket-authorization switch in MobileHostService+TicketAuthorization.swift uses an explicit four-method list. Any new mac.power.* method added to the handler but omitted from the ticket-auth list falls to the default case and bypasses the mac-wide ticket check on the .stackBearer path. The two lists are structurally asymmetric: one is implicitly open, the other is explicitly closed. A comment or shared constant set documenting the intended scope of mac.power.* methods would prevent the two enforcement points from silently diverging.

Comment on lines +1094 to +1096

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove redundant exact-match check.

The method == "mac.power.status" check is redundant because "mac.power.status".hasPrefix("mac.power.") will already evaluate to true.

💡 Proposed refactor
-    nonisolated private static func isPrivilegedMacPowerMethod(_ method: String) -> Bool {
-        method == "mac.power.status" || method.hasPrefix("mac.power.")
-    }
+    nonisolated private static func isPrivilegedMacPowerMethod(_ method: String) -> Bool {
+        method.hasPrefix("mac.power.")
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
nonisolated private static func isPrivilegedMacPowerMethod(_ method: String) -> Bool {
method == "mac.power.status" || method.hasPrefix("mac.power.")
}
nonisolated private static func isPrivilegedMacPowerMethod(_ method: String) -> Bool {
method.hasPrefix("mac.power.")
}
🤖 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/Mobile/MobileHostService.swift` around lines 1094 - 1096, Update
isPrivilegedMacPowerMethod to remove the redundant exact-match comparison and
rely solely on method.hasPrefix("mac.power.").


nonisolated static func connectionStatusResult(
for request: MobileHostRPCRequest,
authorization: MobileHostConnectionAuthorizationContext,
Expand Down
33 changes: 33 additions & 0 deletions Sources/PowerAssertionHolder.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import Foundation
import IOKit.pwr_mgt

@MainActor
final class PowerAssertionHolder: PowerAssertionHolding {
enum HolderError: Error { case createFailed(IOReturn) }

private var assertionID = IOPMAssertionID(0)
var isEnabled: Bool { assertionID != 0 }

deinit {
if assertionID != 0 { IOPMAssertionRelease(assertionID) }
}
Comment on lines +8 to +13

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 | 🟠 Major | ⚡ Quick win

Fix Swift 6 strict concurrency violation in deinit.

In Swift 6, deinit for actor-isolated types is evaluated as nonisolated because it can theoretically execute on any thread when the last reference drops. Accessing the @MainActor-isolated var assertionID inside deinit will trigger a strict concurrency compiler error.

Since assertionID is a simple primitive that is only safely mutated on the main actor and deinit runs exclusively after all other references are gone, you can resolve the isolation error by marking the property nonisolated(unsafe).

🛠 Proposed fix
-    private var assertionID = IOPMAssertionID(0)
+    private nonisolated(unsafe) var assertionID = IOPMAssertionID(0)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private var assertionID = IOPMAssertionID(0)
var isEnabled: Bool { assertionID != 0 }
deinit {
if assertionID != 0 { IOPMAssertionRelease(assertionID) }
}
private nonisolated(unsafe) var assertionID = IOPMAssertionID(0)
var isEnabled: Bool { assertionID != 0 }
deinit {
if assertionID != 0 { IOPMAssertionRelease(assertionID) }
}
🤖 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/PowerAssertionHolder.swift` around lines 8 - 13, Mark the assertionID
property in PowerAssertionHolder as nonisolated(unsafe) so deinit can access it
without violating Swift 6 strict concurrency isolation. Keep its existing type,
initialization, and usage unchanged.


func setEnabled(_ enabled: Bool) throws {
guard enabled != isEnabled else { return }
if enabled {
var newID = IOPMAssertionID(0)
let result = IOPMAssertionCreateWithName(
kIOPMAssertionTypePreventUserIdleSystemSleep as CFString,
IOPMAssertionLevel(kIOPMAssertionLevelOn),
"cmux remote keep-awake" as CFString,
&newID
)
guard result == kIOReturnSuccess else { throw HolderError.createFailed(result) }
assertionID = newID
} else {
let result = IOPMAssertionRelease(assertionID)
guard result == kIOReturnSuccess else { throw HolderError.createFailed(result) }
assertionID = IOPMAssertionID(0)
}
}
}
7 changes: 7 additions & 0 deletions Sources/PowerAssertionHolding.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import Foundation

@MainActor
protocol PowerAssertionHolding: AnyObject {
var isEnabled: Bool { get }
func setEnabled(_ enabled: Bool) throws
}
Loading