Repository navigation
iOS: control Mac sleep & caffeinate from cmux mobile - #6902
austinywang wants to merge 23 commits into
Conversation
Adds Mac power control to the phone's per-computer detail screen so a paired phone can sleep the Mac, disable active keep-awake (caffeinate), and see whether the Mac is currently being kept awake and by which processes. Resolves the three controls in #6482. - New CmuxMacPower package (macOS): a command-runner seam, a `pmset -g assertions` parser, and MacPowerController (sleep via osascript→System Events, keep-awake status, disable via `pkill -x caffeinate`), all behind an injected runner with Swift Testing unit tests. - Mobile host RPC: mac.power.status / mac.power.sleep / mac.power.keep_awake.disable, advertised by the new mac.power.control.v1 capability. These ride the existing same-account data-plane trust boundary (the one that already gates terminal.input), so they need no extra gate. - iOS client methods + a "Mac Power" section in MacComputerDetailView, with a sleep confirmation dialog and a live keep-awake summary. A sleep request that drops the connection is treated as success (the Mac slept); an explicit RPC error surfaces an Automation-permission hint. All strings localized (en/ja). Fixes #6482 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new macOS Swift package (CmuxMacPower) providing pmset-based keep-awake status parsing, sleep, and caffeinate-disable controls; wires these into host RPC dispatch and capability advertisement; extends the iOS mobile shell client with typed models, error classification, and RPC routing; and adds a Mac Power section to the Mac detail view with related localizations and tests. ChangesMac power control
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MacComputerDetailView
participant MobileShellComposite
participant TerminalController
participant MacPowerController
participant pmset/osascript/kill
MacComputerDetailView->>MobileShellComposite: sleepMac(macDeviceID)
MobileShellComposite->>TerminalController: RPC mac.power.sleep
TerminalController->>MacPowerController: sleepSystem()
MacPowerController->>pmset/osascript/kill: osascript sleep command
pmset/osascript/kill-->>MacPowerController: exit status
MacPowerController-->>TerminalController: Bool success
TerminalController-->>MobileShellComposite: ok / sleep_failed
MobileShellComposite-->>MacComputerDetailView: MobileMacSleepResult (requested/refused/failed)
sequenceDiagram
participant MacComputerDetailView
participant MobileShellComposite
participant TerminalController
participant MacPowerController
participant pmset/osascript/kill
MacComputerDetailView->>MobileShellComposite: disableMacKeepAwake(macDeviceID)
MobileShellComposite->>TerminalController: RPC mac.power.keep_awake.disable
TerminalController->>MacPowerController: disableKeepAwake()
MacPowerController->>pmset/osascript/kill: ps check + kill caffeinate PIDs
MacPowerController->>pmset/osascript/kill: re-read pmset assertions
pmset/osascript/kill-->>MacPowerController: refreshed status
MacPowerController-->>TerminalController: MacKeepAwakeDisableOutcome
TerminalController-->>MobileShellComposite: terminated_caffeinate + status json
MobileShellComposite-->>MacComputerDetailView: MobileMacPowerStatus
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (22 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Key the auto-load .task on both isForeground and supportsMacPowerControl so the first keep-awake status fetch fires when the host capability handshake flips the section on while the detail view is already open (was keyed to isForeground alone, leaving the section stuck on 'Checking…'). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e-should-control-mac-sleep # Conflicts: # .github/swift-file-length-budget.tsv
Greptile SummaryThis PR adds Mac power control to the cmux mobile app — sleep the Mac, disable caffeinate, and show keep-awake status — backed by a new
Confidence Score: 4/5Safe to merge after addressing the blocking-read issue in the process launcher. The Packages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerProcessLauncher.swift — the stdout reader should move off the cooperative pool onto a background GCD queue. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant iOS as iOS (MacComputerDetailView)
participant Shell as MobileShellComposite
participant RPC as TerminalController (Mac)
participant Ctrl as MacPowerController
participant OS as macOS (pmset / osascript / kill)
iOS->>Shell: macPowerStatus(macDeviceID:)
Shell->>RPC: mac.power.status RPC
RPC->>Ctrl: keepAwakeStatus()
Ctrl->>OS: /usr/bin/pmset -g assertions
OS-->>Ctrl: output (or nil on timeout)
Ctrl-->>RPC: MacKeepAwakeStatus?
RPC-->>Shell: JSON status (or status_unavailable error)
Shell-->>iOS: MobileMacPowerStatus?
iOS->>Shell: sleepMac(macDeviceID:)
Shell->>RPC: mac.power.sleep RPC
RPC->>Ctrl: sleepSystem()
Ctrl->>OS: osascript tell System Events to sleep
OS-->>Ctrl: exit 0 or non-zero if Automation denied
RPC-->>Shell: ok / sleep_failed error
Shell-->>iOS: MobileMacSleepResult
iOS->>Shell: disableMacKeepAwake(macDeviceID:)
Shell->>RPC: mac.power.keep_awake.disable RPC
RPC->>Ctrl: disableKeepAwake()
Ctrl->>OS: pmset status before
Ctrl->>OS: ps revalidate PID
Ctrl->>OS: kill SIGTERM
Ctrl->>OS: pmset status after
RPC-->>Shell: terminated_caffeinate + status
Shell-->>iOS: MobileMacPowerStatus?
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant iOS as iOS (MacComputerDetailView)
participant Shell as MobileShellComposite
participant RPC as TerminalController (Mac)
participant Ctrl as MacPowerController
participant OS as macOS (pmset / osascript / kill)
iOS->>Shell: macPowerStatus(macDeviceID:)
Shell->>RPC: mac.power.status RPC
RPC->>Ctrl: keepAwakeStatus()
Ctrl->>OS: /usr/bin/pmset -g assertions
OS-->>Ctrl: output (or nil on timeout)
Ctrl-->>RPC: MacKeepAwakeStatus?
RPC-->>Shell: JSON status (or status_unavailable error)
Shell-->>iOS: MobileMacPowerStatus?
iOS->>Shell: sleepMac(macDeviceID:)
Shell->>RPC: mac.power.sleep RPC
RPC->>Ctrl: sleepSystem()
Ctrl->>OS: osascript tell System Events to sleep
OS-->>Ctrl: exit 0 or non-zero if Automation denied
RPC-->>Shell: ok / sleep_failed error
Shell-->>iOS: MobileMacSleepResult
iOS->>Shell: disableMacKeepAwake(macDeviceID:)
Shell->>RPC: mac.power.keep_awake.disable RPC
RPC->>Ctrl: disableKeepAwake()
Ctrl->>OS: pmset status before
Ctrl->>OS: ps revalidate PID
Ctrl->>OS: kill SIGTERM
Ctrl->>OS: pmset status after
RPC-->>Shell: terminated_caffeinate + status
Shell-->>iOS: MobileMacPowerStatus?
Reviews (17): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// merge multiple lines for the same pid, and derive the booleans from the set | ||
| /// of assertion types seen. The system-wide aggregate counts are intentionally | ||
| /// ignored because they cannot attribute "who" is keeping the Mac awake. | ||
| public enum MacKeepAwakeStatusParser { |
There was a problem hiding this comment.
MacKeepAwakeStatusParser is a caseless enum whose entire API is static methods and static lets — exactly the shape the cmux no-ambient-global-state rule explicitly flags. Caseless enums cannot be instantiated, so the type cannot be injected, subclassed, or mocked independently of the static state. The canonical fix is to convert it to a struct (or fold parse(_:) into MacPowerController directly, since the controller is already the only caller).
Rule Used: Flag new ambient global state in production Swift:... (source)
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!
There was a problem hiding this comment.
Addressed: parser is now MacKeepAwakeStatus.parse(pmsetAssertions:) with private file-scoped helpers; no caseless namespace enum remains.
— Claude Code
| public func keepAwakeStatus() async -> MacKeepAwakeStatus { | ||
| guard let output = await runner.capture("/usr/bin/pmset", ["-g", "assertions"]) else { | ||
| return .idle | ||
| } | ||
| return MacKeepAwakeStatusParser.parse(output) |
There was a problem hiding this comment.
When runner.capture returns nil (e.g., the process fails to launch or pmset exits non-zero on a restricted system), keepAwakeStatus() returns .idle — all flags false, empty holders. The macOS RPC then sends {"kept_awake": false, …} to iOS, and the phone renders "Sleeps normally." The user sees a clean status when the actual sleep-prevention state is simply unknown. The reliability rule requires failing closed: on a read failure, the status should be distinguishable from a confirmed-idle state (e.g., return an Optional<MacKeepAwakeStatus> and surface an error banner on iOS), rather than substituting a fake all-clear.
Rule Used: Flag correctness-critical detection/identity deriv... (source)
There was a problem hiding this comment.
Addressed: keepAwakeStatus() now returns nil on pmset capture failure or timeout; the RPC maps that to status_unavailable, and iOS surfaces a localized status-unavailable message instead of Sleeps normally.
— Claude Code
There was a problem hiding this comment.
Addressed earlier: keepAwakeStatus() no longer turns a pmset/capture failure into a confirmed idle status. The controller returns nil for an unreadable status, and the mobile UI surfaces the unavailable/read-failed state instead of showing "Sleeps normally".
— Claude Code
| public init(from decoder: any Decoder) throws { | ||
| let container = try decoder.container(keyedBy: CodingKeys.self) | ||
| pid = (try container.decodeIfPresent(Int.self, forKey: .pid)) ?? 0 | ||
| processName = (try container.decodeIfPresent(String.self, forKey: .processName)) ?? "" |
There was a problem hiding this comment.
pid: 0 fallback creates duplicate Identifiable IDs for missing-pid holders
MobileMacPowerHolder.id is pid, but when the pid field is absent from JSON the decoder assigns 0, so every such holder shares id = 0. SwiftUI ForEach treats duplicate IDs as the same element and will silently drop or misrender extra rows. Using processName as a fallback (or a composite of pid + processName) keeps identity unique in practice even if the server ever omits the pid field.
| public init(from decoder: any Decoder) throws { | |
| let container = try decoder.container(keyedBy: CodingKeys.self) | |
| pid = (try container.decodeIfPresent(Int.self, forKey: .pid)) ?? 0 | |
| processName = (try container.decodeIfPresent(String.self, forKey: .processName)) ?? "" | |
| public init(from decoder: any Decoder) throws { | |
| let container = try decoder.container(keyedBy: CodingKeys.self) | |
| processName = (try container.decodeIfPresent(String.self, forKey: .processName)) ?? "" | |
| pid = (try container.decodeIfPresent(Int.self, forKey: .pid)) ?? processName.hashValue |
There was a problem hiding this comment.
Addressed: MobileMacPowerHolder.id is now a deterministic composite of pid, process, assertion types, and detail, with regression coverage for missing-pid holders.
— Claude Code
The package-conventions lint forbids caseless-enum namespace types. Replace the all-static `enum MacKeepAwakeStatusParser` with a `MacKeepAwakeStatus.parse(pmsetAssertions:)` static factory on the receiver value type plus file-scoped private helpers — no behavior change, 16 tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
…e-should-control-mac-sleep # Conflicts: # .github/swift-file-length-budget.tsv
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+MacPower.swift:
- Around line 139-161: The sleepMac flow is treating request construction
failures as a successful sleep request because MobileCoreRPCClient.requestData
and client.sendRequest are wrapped in the same do/catch. Split the
request-building step from the send step in
MobileShellComposite+MacPower.sleepMac so only post-send disconnects can map to
.requested, and make any failure from requestData return .failed (or otherwise
not .requested). Keep the existing handling for
disconnectForAuthorizationFailureIfNeeded and
MobileShellConnectionError.rpcError unchanged.
- Around line 5-8: The `sleepMac` flow is conflating request-construction
failures from `requestData(...)` with the expected post-sleep disconnect path in
`sendRequest(...)`, so non-RPC errors are being reported as `.requested`.
Refactor the `sleepMac` implementation to build the request outside the
success/failure translation or add separate error handling so only the expected
disconnect maps to `.requested`, while local request-building failures return
`.failed` instead.
In
`@Packages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeStatus`+Parse.swift:
- Around line 113-153: macParsePmsetProcessLine currently parses the process
head with the first opening and closing parentheses, which breaks names that
contain embedded parentheses and can corrupt the remainder parsing. Update the
name extraction in macParsePmsetProcessLine to identify the end of the `pid
N(name):` header by matching the literal `): ` sequence instead of the first
`)`, then derive `name` and `remainder` from that boundary so assertion types
and `named:` detail are preserved for helpers like `Google Chrome` and similar
process names.
In
`@Packages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerCommandRunning.swift`:
- Around line 22-79: The current SystemMacPowerCommandRunner.runSync path still
waits indefinitely on Process.waitUntilExit(), so Mac power RPCs like
v2MacPowerSleep can hang forever if the underlying tool stalls. Add a bounded
timeout inside runSync for both run(_:_:) and capture(_:_:), and if the process
exceeds it, terminate/kill the Process and return failure so sleepSystem() and
its callers can fail cleanly instead of blocking. Keep the change localized to
SystemMacPowerCommandRunner and its runSync helper so the timeout applies
consistently to all command executions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81c972f2-14eb-4ce1-a5bb-6fb0d9a7f4c8
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (16)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+MacPower.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerDetailView.swiftPackages/macOS/CmuxMacPower/Package.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeStatus+Parse.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeStatus.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerCommandRunning.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerController.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/MacKeepAwakeStatusParserTests.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/MacPowerControllerTests.swiftSources/Mobile/MobileHostService+Capabilities.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmux.xcworkspace/contents.xcworkspacedatacmuxTests/MobileHostAuthorizationTests.swiftios/cmux/Resources/Localizable.xcstrings
…e-should-control-mac-sleep
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerDetailView.swift (1)
404-406: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
String.localizedStringWithFormatfor the localized format string.
String(format:)doesn't apply locale-aware substitution/positional-argument handling that a translated format may need. For UI-facing localized formats this repo's convention isString.localizedStringWithFormat.♻️ Proposed change
- return String( - format: L10n.string("mobile.computers.power.byFormat", defaultValue: "Kept awake by %@"), - names.joined(separator: ", ")) + return String.localizedStringWithFormat( + L10n.string("mobile.computers.power.byFormat", defaultValue: "Kept awake by %@"), + names.joined(separator: ", "))Based on learnings that localized
%@-style format strings taking separate format arguments should useString.localizedStringWithFormatrather thanString(format:).🤖 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 404 - 406, The localized format in MacComputerDetailView’s power-by label is using String(format:) instead of the repo’s preferred locale-aware formatter. Update the return in the power-by formatting path to use String.localizedStringWithFormat with the L10n.string("mobile.computers.power.byFormat", ...) value and the joined names argument, so translated %@ formatting is handled correctly.Source: Learnings
🤖 Prompt for all review comments with 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.
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerDetailView.swift`:
- Around line 404-406: The localized format in MacComputerDetailView’s power-by
label is using String(format:) instead of the repo’s preferred locale-aware
formatter. Update the return in the power-by formatting path to use
String.localizedStringWithFormat with the
L10n.string("mobile.computers.power.byFormat", ...) value and the joined names
argument, so translated %@ formatting is handled correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd8d52f3-6c55-4e19-9246-462e2e14c4a5
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (11)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+MacPower.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileMacPowerHolderTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerDetailView.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeStatus+Parse.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerCommandRunning.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerController.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/MacKeepAwakeStatusParserTests.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/MacPowerControllerTests.swiftResources/Localizable.xcstringsSources/TerminalController.swiftios/cmux/Resources/Localizable.xcstrings
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@Packages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeStatus`+Parse.swift:
- Around line 3-23: The parsing entry point was moved from a method on
MacKeepAwakeStatus to a top-level function, which violates the
no-ambient-global-state guideline for shared production APIs. Move
macParseKeepAwakeStatus back onto MacKeepAwakeStatus as a static (or instance)
parse method, then update all call sites such as MacPowerController and the test
suite to call through MacKeepAwakeStatus instead of the free function.
In
`@Packages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerCommandProcess.swift`:
- Line 20: The MacPowerCommandProcess setup is discarding all stderr from the
spawned system tools by routing standardError to FileHandle.nullDevice, which
prevents useful diagnostics from pmset, kill, or ps failures. Update
MacPowerCommandProcess so stderr is preserved or captured instead of sending it
to /dev/null, and make sure the caller can access or log that error output when
commands fail.
- Around line 11-86: `macPowerRunProcess` is currently a top-level internal
function being used across files, which violates the guideline against exposing
ambient free-function APIs. Move the behavior into an owning type such as a new
`MacPowerProcessLauncher` or onto `SystemMacPowerCommandRunner` itself, and
update all call sites to use that method instead of the free function. Keep
`macPowerReadToEnd` and `macPowerScheduleSigkill` private helpers, and preserve
the existing process/timeout logic inside the new method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 74541fb1-559c-4f5f-8d9a-c7fb779305bb
📒 Files selected for processing (17)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileMacPowerHolder.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileMacPowerStatus.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileMacSleepResult.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+MacPower.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeDisableOutcome.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeStatus+Parse.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeStatus.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerAssertionHolder.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerCommandProcess.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerCommandRunning.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerController.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerProcessRunState.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/SystemMacPowerCommandRunner.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/FakeMacPowerCommandRunner.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/FakeMacPowerCommandRunnerCall.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/MacKeepAwakeStatusParserTests.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/MacPowerControllerTests.swift
💤 Files with no reviewable changes (1)
- Packages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerCommandRunning.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+MacPower.swift (1)
10-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
terminated_caffeinateis documented but never decoded.The doc comment says the payload is
{ terminated_caffeinate, status }, butMobileMacKeepAwakeDisableResponseonly decodesstatus. If callers/UI ever need to confirm caffeinate was actually terminated (vs. some other keep-awake source), that signal is silently dropped.Also applies to: 83-84
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+MacPower.swift around lines 10 - 17, The `MobileMacKeepAwakeDisableResponse` model currently documents a `terminated_caffeinate` field but only decodes `status`, so the payload signal is being dropped. Update this response type to include and decode `terminated_caffeinate` alongside `status`, and keep the wrapper/comment aligned with the actual `mac.power.keep_awake.disable` payload so callers can inspect whether caffeinate was terminated.Packages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerController.swift (2)
49-66: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the termination result when the final status reread fails. If the post-kill
keepAwakeStatus()call returnsnil, this method dropsterminatedCaffeinatetoo and makes a successful disable indistinguishable from an unavailable status read. Return an outcome that can carry an unknown status instead of collapsing the whole call tonil.🤖 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/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerController.swift` around lines 49 - 66, MacPowerController.disableKeepAwake currently returns nil when the post-kill keepAwakeStatus() reread fails, which loses the already-known terminatedCaffeinate result. Update disableKeepAwake() to preserve the termination outcome even if the final status fetch is unavailable, using MacKeepAwakeDisableOutcome with an unknown/missing status representation instead of collapsing the whole method to nil. Use the existing disableKeepAwake(), keepAwakeStatus(), and MacKeepAwakeDisableOutcome symbols to keep the successful kill signal result intact.
29-43: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
sleepSystem()needs its own timeout budget. It reuses the shared 10sSystemMacPowerCommandRunnerdeadline, so the first Automation grant can still be open when the runner killsosascriptand reports a failure. Give sleep a separate, longer timeout instead of sharing the fast-status default.🤖 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/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerController.swift` around lines 29 - 43, sleepSystem() is currently using the shared SystemMacPowerCommandRunner timeout, which is too short for the first Automation grant flow; update MacPowerController.sleepSystem() to run with its own longer timeout budget instead of the default fast-status deadline. Adjust the call site in sleepSystem() to use a dedicated timeout/configuration path in SystemMacPowerCommandRunner so osascript can complete when Automation permission prompts are involved, while leaving the existing short timeout behavior unchanged for other commands.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+MacPower.swift:
- Around line 10-17: The `MobileMacKeepAwakeDisableResponse` model currently
documents a `terminated_caffeinate` field but only decodes `status`, so the
payload signal is being dropped. Update this response type to include and decode
`terminated_caffeinate` alongside `status`, and keep the wrapper/comment aligned
with the actual `mac.power.keep_awake.disable` payload so callers can inspect
whether caffeinate was terminated.
In `@Packages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerController.swift`:
- Around line 49-66: MacPowerController.disableKeepAwake currently returns nil
when the post-kill keepAwakeStatus() reread fails, which loses the already-known
terminatedCaffeinate result. Update disableKeepAwake() to preserve the
termination outcome even if the final status fetch is unavailable, using
MacKeepAwakeDisableOutcome with an unknown/missing status representation instead
of collapsing the whole method to nil. Use the existing disableKeepAwake(),
keepAwakeStatus(), and MacKeepAwakeDisableOutcome symbols to keep the successful
kill signal result intact.
- Around line 29-43: sleepSystem() is currently using the shared
SystemMacPowerCommandRunner timeout, which is too short for the first Automation
grant flow; update MacPowerController.sleepSystem() to run with its own longer
timeout budget instead of the default fast-status deadline. Adjust the call site
in sleepSystem() to use a dedicated timeout/configuration path in
SystemMacPowerCommandRunner so osascript can complete when Automation permission
prompts are involved, while leaving the existing short timeout behavior
unchanged for other commands.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 487479e3-ab60-4fb8-9a9d-3604f50a28fd
📒 Files selected for processing (9)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileMacSleepErrorClassifier.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileMacSleepResult.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+MacPower.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileMacSleepResultTests.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacKeepAwakeStatusParser.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerController.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/MacPowerProcessLauncher.swiftPackages/macOS/CmuxMacPower/Sources/CmuxMacPower/SystemMacPowerCommandRunner.swiftPackages/macOS/CmuxMacPower/Tests/CmuxMacPowerTests/MacKeepAwakeStatusParserTests.swift
| internal import CmuxMobileRPC | ||
|
|
||
| struct MobileMacSleepErrorClassifier { | ||
| func result(forSendError error: any Error) -> MobileMacSleepResult { | ||
| guard let connectionError = error as? MobileShellConnectionError else { | ||
| return .failed | ||
| } | ||
| switch connectionError { | ||
| case .rpcError: | ||
| return .refused | ||
| case .invalidResponse, | ||
| .connectionClosed, | ||
| .requestTimedOut, | ||
| .insecureManualRoute, | ||
| .attachTicketExpired, | ||
| .authorizationFailed, | ||
| .accountMismatch: | ||
| return .failed | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The PR description states "A sleep request that drops the connection is treated as success (the Mac slept and the link died)", but MobileMacSleepErrorClassifier maps both .connectionClosed and .requestTimedOut to .failed. In practice tell application "System Events" to sleep causes osascript to return before the Mac actually sleeps, so the success response is usually delivered in time — but there is a real race window where the Mac can go to sleep before the TCP frame carrying the RPC response is flushed. When that race fires, the phone shows "Couldn't reach the Mac to sleep it." even though the Mac successfully went to sleep. .connectionClosed and .requestTimedOut arriving in direct response to a sleep command are the primary evidence of success, not failure, and should return .requested.
There was a problem hiding this comment.
I am leaving this as .failed intentionally. The branch now has MobileMacSleepErrorClassifier tests asserting that connectionClosed and requestTimedOut are delivery failures, while only an explicit successful RPC response maps to .requested and an RPC error maps to .refused. A closed/timed-out transport after send is ambiguous from the phone's side; osascript normally returns before actual sleep, so treating those as success creates false positives. The older PR wording about dropped connections was stale after the sleep-error tightening commits.
— Claude Code
There was a problem hiding this comment.
That's a fair correction — if osascript returns before the actual sleep, the response window is reliable enough that a transport failure genuinely is ambiguous (dropped wifi, flaky link, etc.) rather than a sleep signal. The test-backed decision to require an explicit success response makes sense.
Withdrawing the concern.
…e-should-control-mac-sleep # Conflicts: # .github/swift-file-length-budget.tsv # Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Fixes #6482
What
Lets the cmux mobile app control the connected Mac's power state, covering the three controls the issue asks for:
The controls live in the per-computer detail screen (Computers → a Mac → Mac Power), shown only for the Mac the phone is actively connected to and only when that Mac advertises the capability.
How
CmuxMacPower(new macOS package, unit-tested):MacPowerCommandRunningseam (+SystemMacPowerCommandRunneroverProcess) so the logic is testable without touching the real machine.MacKeepAwakeStatusParser— parsespmset -g assertionsinto a structured status (who holds which sleep-prevention assertion; flags for cmux / caffeinate / system vs display).MacPowerController—sleepSystem()(AppleScriptSystem Events → sleep, the supported no-root path; cmux already ships the apple-events entitlement + usage string),keepAwakeStatus(), anddisableKeepAwake()(pkill -x caffeinate, then re-read).Mobile host RPC (macOS app):
mac.power.status,mac.power.sleep,mac.power.keep_awake.disableinmobileHostHandleRPC, advertised via the newmac.power.control.v1capability.terminal.input), so no extra gate is added.iOS:
supportsMacPowerControlgate +macPowerStatus/sleepMac/disableMacKeepAwakeclient methods (Decodable status mirror).MacComputerDetailViewwith a sleep confirmation and a live keep-awake summary. A sleep request that drops the connection is treated as success (the Mac slept and the link died); an explicit RPC error surfaces an Automation-permission hint. All strings localized (en/ja).Notes
caffeinateis terminated.🤖 Generated with Claude Code
Summary by cubic
Adds Mac power controls to the cmux mobile app so you can sleep the Mac, stop
caffeinate, and see who’s keeping it awake, with clear errors when status can’t be read or sleep is refused. ShipsCmuxMacPower, mobile RPCs, and an iOS “Mac Power” section to satisfy #6482.New Features
mac.power.control.v1) with a keep‑awake summary and holders, Sleep with confirm, Disable keep‑awake, and Refresh. Auto‑loads when the capability arrives, hardens busy state, ensures stable row identity without PIDs (with a regression test), and shows “Couldn't read Mac power status.” when unavailable. Sleep outcomes classify explicit refusal vs delivery failures (timeouts/disconnects are failures).CmuxMacPowerprovides apmset -g assertionsparser (handles system‑wide counts when no owning‑process lines exist) andMacPowerControllerto sleep via System Events, read status, and disable keep‑awake by signaling only verifiedcaffeinatePIDs (revalidated viaps). Includes a command runner with timeouts, SIGTERM→SIGKILL handling, and a safeguard to avoid sending SIGKILL to a reused PID after timeouts. Unit tests cover the parser, parentheses regression, timeouts/PID reuse, and PID revalidation.mac.power.status,mac.power.sleep, andmac.power.keep_awake.disable, advertised viamac.power.control.v1. RPCs return explicit errors for status unavailability and sleep refusal so iOS can show the right message.Migration
Written for commit 2b8d8a6. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes