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
34 changes: 34 additions & 0 deletions CLI/CMUXCLI+WorkspaceTodo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ extension CMUXCLI {
client: SocketClient,
windowOverride: String?
) throws -> (params: [String: Any], rest: [String]) {
try validateWorkspaceTodoSelectorOption(commandArgs, name: "--workspace")
try validateWorkspaceTodoSelectorOption(commandArgs, name: "--window")
let (workspaceArg, rem0) = parseOption(commandArgs, name: "--workspace")
let (windowArg, rem1) = parseOption(rem0, name: "--window")
var params: [String: Any] = [:]
Expand All @@ -31,6 +33,38 @@ extension CMUXCLI {
return (params, rest)
}

private func validateWorkspaceTodoSelectorOption(_ args: [String], name: String) throws {
var pastTerminator = false
for (index, arg) in args.enumerated() {
if arg == "--" {
pastTerminator = true
continue
}
guard !pastTerminator else { continue }
if arg.hasPrefix("\(name)=") {
let value = String(arg.dropFirst(name.count + 1))
try validateWorkspaceTodoSelectorValue(value, name: name)
continue
}
if arg == name {
guard index + 1 < args.count else {
throw CLIError(message: "\(name) requires a non-empty value")
}
let value = args[index + 1]
guard !value.hasPrefix("--") else {
throw CLIError(message: "\(name) requires a non-empty value")
}
try validateWorkspaceTodoSelectorValue(value, name: name)
}
}
}

private func validateWorkspaceTodoSelectorValue(_ value: String, name: String) throws {
guard !value.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
throw CLIError(message: "\(name) requires a non-empty value")
}
}

/// Parses a checklist item selector: a UUID id, or a 1-based index as
/// printed by `cmux todo list` (sent as the wire's 0-based `index`).
private func workspaceTodoItemSelectorParams(_ raw: String) throws -> [String: Any] {
Expand Down
3 changes: 3 additions & 0 deletions Packages/macOS/CmuxControlSocket/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ let package = Package(
],
dependencies: [
.package(path: "../CmuxSettings"),
.package(path: "../CmuxWorkspaces"),
],
targets: [
.target(
name: "CmuxControlSocket",
dependencies: [
.product(name: "CmuxSettings", package: "CmuxSettings"),
.product(name: "CmuxWorkspaces", package: "CmuxWorkspaces"),
],
swiftSettings: [
.swiftLanguageMode(.v6),
Expand All @@ -33,6 +35,7 @@ let package = Package(
dependencies: [
"CmuxControlSocket",
.product(name: "CmuxSettings", package: "CmuxSettings"),
.product(name: "CmuxWorkspaces", package: "CmuxWorkspaces"),
]
),
]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
internal import Foundation
internal import CmuxWorkspaces

/// Workspace-todo set/open verbs extracted from the primary workspace-todo coordinator file, which sits at its file-length budget.
extension ControlCommandCoordinator {
Expand All @@ -16,6 +17,9 @@ extension ControlCommandCoordinator {
guard case .array(let rawItems)? = params["items"] else {
return .invalid(.err(code: "invalid_params", message: "Missing or invalid items", data: nil))
}
guard rawItems.count <= WorkspaceChecklistItem.maxChecklistItems else {
return .invalid(workspaceTodoSetTooManyItemsError(count: rawItems.count))
}
var items: [ControlWorkspaceTodoSetItemParam] = []
items.reserveCapacity(rawItems.count)
for (index, rawItem) in rawItems.enumerated() {
Expand Down Expand Up @@ -88,11 +92,7 @@ extension ControlCommandCoordinator {
data: .object(["index": .int(Int64(index))])
)
case .tooManyItems(let count):
return .err(
code: "invalid_params",
message: "items exceeds the checklist cap of 50",
data: .object(["count": .int(Int64(count))])
)
return workspaceTodoSetTooManyItemsError(count: count)
case .invalidState(let raw):
return .err(
code: "invalid_params",
Expand All @@ -110,6 +110,14 @@ extension ControlCommandCoordinator {
}
}

private func workspaceTodoSetTooManyItemsError(count: Int) -> ControlCallResult {
.err(
code: "invalid_params",
message: "items exceeds the checklist cap of \(WorkspaceChecklistItem.maxChecklistItems)",
data: .object(["count": .int(Int64(count))])
)
}

/// `workspace.todo.open` β€” open (or focus) the workspace's todo pane.
func workspaceTodoOpen(_ params: [String: JSONValue]) -> ControlCallResult {
let resolution = context?.controlWorkspaceTodoOpen(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import CmuxWorkspaces
import Testing
@testable import CmuxControlSocket

Expand Down Expand Up @@ -95,6 +96,24 @@ extension ControlCommandCoordinatorWorkspaceTodoTests {
#expect(context.lastSetItems == nil)
}

@Test func todoSetOverCapRejectsBeforeParsingItems() throws {
let (coordinator, context) = makeCoordinator()
let rawItems = (0...WorkspaceChecklistItem.maxChecklistItems).map { _ in
JSONValue.object(["text": .string("x")])
}
let result = try #require(coordinator.handle(request("workspace.todo.set", [
"items": .array(rawItems),
])))
guard case .err(let code, let message, let data) = result else {
Issue.record("expected err, got \(result)")
return
}
#expect(code == "invalid_params")
#expect(message == "items exceeds the checklist cap of \(WorkspaceChecklistItem.maxChecklistItems)")
#expect(data == .object(["count": .int(Int64(WorkspaceChecklistItem.maxChecklistItems + 1))]))
#expect(context.lastSetItems == nil)
}

@Test func todoSetEchoesAtomicRejections() throws {
let (coordinator, context) = makeCoordinator()
context.setResolution = .emptyText(index: 2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,8 @@ extension ShortcutAction {
case .renameTab: return ShortcutStroke(key: "r", command: true)
case .renameWorkspace: return ShortcutStroke(key: "r", command: true, shift: true)
case .editWorkspaceDescription: return ShortcutStroke(key: "e", command: true, option: true)
// Cmd+; pins the status to done; the Cmd-"D" family is taken by
// split/diff actions and Cmd+Ctrl+D is macOS-reserved. Mirrors the
// app-side table.
case .markWorkspaceDone: return ShortcutStroke(key: ";", command: true)
case .cycleWorkspaceStatus: return ShortcutStroke(key: ";", command: true, shift: true)
case .markWorkspaceDone: return ShortcutStroke(key: ";", command: true, control: true)
case .cycleWorkspaceStatus: return ShortcutStroke(key: ";", command: true, shift: true, control: true)
case .toggleChecklistItemComplete: return ShortcutStroke(key: "\r", command: true)
case .closeTab: return ShortcutStroke(key: "w", command: true)
case .closeOtherTabsInPane: return ShortcutStroke(key: "t", command: true, option: true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ extension ShortcutAction {
self != .fileExplorerOpenSelection
&& self != .fileExplorerOpenSelectionFinderAlias
&& self != .cycleTextBoxSubmitAction
&& self != .toggleChecklistItemComplete
}

/// The action's built-in focus context expressed as a ``ShortcutWhenClause``,
Expand Down
31 changes: 25 additions & 6 deletions Sources/AppDelegate+DockSurfaceMove.swift
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ extension AppDelegate {
focus: Bool,
focusWindow: Bool
) -> Bool {
guard let panel = sourceDock.panels[panelId],
canTransferSurfaceAcrossWorkspaceBoundary(panel: panel) else {
return false
}
guard let destinationManager = tabManagerFor(tabId: targetWorkspaceId),
let destinationWorkspace = destinationManager.tabs.first(where: { $0.id == targetWorkspaceId }) else {
return false
Expand Down Expand Up @@ -232,6 +236,10 @@ extension AppDelegate {
focus: Bool = true,
focusWindow: Bool = false
) -> Bool {
guard let panel = sourceDock.panels[panelId],
canTransferSurfaceAcrossWorkspaceBoundary(panel: panel) else {
return false
}
// A window Dock resolves its owning window; a Workspace Dock resolves
// that workspace's window (see `dockReferenceTabManager`).
guard let manager = dockReferenceTabManager(for: sourceDock) else { return false }
Expand Down Expand Up @@ -266,12 +274,23 @@ extension AppDelegate {
}

private func canMoveSurfaceIntoDock(_ source: ContainerSurfaceLocation) -> Bool {
if case .workspace(_, let workspace, _, _) = source,
workspace.isRemoteTmuxMirror {
// Remote tmux mirror panes are manually driven by the mirror
// workspace. Dock has no mirror-owned I/O routing yet, so moving one
// would leave the Dock panel detached from its remote owner.
return false
switch source {
case .workspace(_, let workspace, let panelId, _):
guard let panel = workspace.panels[panelId],
canTransferSurfaceAcrossWorkspaceBoundary(panel: panel) else {
return false
}
if workspace.isRemoteTmuxMirror {
// Remote tmux mirror panes are manually driven by the mirror
// workspace. Dock has no mirror-owned I/O routing yet, so moving one
// would leave the Dock panel detached from its remote owner.
return false
}
case .dock(let dock, let panelId):
guard let panel = dock.panels[panelId],
canTransferSurfaceAcrossWorkspaceBoundary(panel: panel) else {
return false
}
}
return true
}
Expand Down
30 changes: 25 additions & 5 deletions Sources/AppDelegate+MoveTabToNewWorkspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,15 @@ struct SurfaceNewWorkspaceMoveResult {

@MainActor
extension AppDelegate {
func canTransferSurfaceAcrossWorkspaceBoundary(panel: any Panel) -> Bool {
panel.panelType != .workspaceTodo
}

func canMoveSurfaceToNewWorkspace(panelId: UUID) -> Bool {
guard let source = locateSurface(surfaceId: panelId),
let sourceWorkspace = source.tabManager.tabs.first(where: { $0.id == source.workspaceId }),
sourceWorkspace.panels[panelId] != nil else {
let panel = sourceWorkspace.panels[panelId],
canTransferSurfaceAcrossWorkspaceBoundary(panel: panel) else {
return false
}
return sourceWorkspace.panels.count > 1
Expand All @@ -29,24 +34,38 @@ extension AppDelegate {
func canMoveBonsplitTab(tabId: UUID, toWorkspace targetWorkspaceId: UUID) -> Bool {
guard let located = locateBonsplitSurface(tabId: tabId),
let sourceWorkspace = located.tabManager.tabs.first(where: { $0.id == located.workspaceId }),
sourceWorkspace.panels[located.panelId] != nil,
let panel = sourceWorkspace.panels[located.panelId],
let destinationManager = tabManagerFor(tabId: targetWorkspaceId),
destinationManager.tabs.contains(where: { $0.id == targetWorkspaceId }) else {
return false
}
if sourceWorkspace.id != targetWorkspaceId,
!canTransferSurfaceAcrossWorkspaceBoundary(panel: panel) {
return false
}
return true
}

func workspaceMoveTargets(forSurface panelId: UUID) -> [WorkspaceMoveTarget] {
guard let source = locateSurface(surfaceId: panelId) else { return [] }
guard let source = locateSurface(surfaceId: panelId),
let sourceWorkspace = source.tabManager.tabs.first(where: { $0.id == source.workspaceId }),
let panel = sourceWorkspace.panels[panelId],
canTransferSurfaceAcrossWorkspaceBoundary(panel: panel) else {
return []
}
return workspaceMoveTargets(
excludingWorkspaceId: source.workspaceId,
referenceWindowId: source.windowId
)
}

func workspaceMoveTargets(forBonsplitTab tabId: UUID) -> [WorkspaceMoveTarget] {
guard let located = locateBonsplitSurface(tabId: tabId) else { return [] }
guard let located = locateBonsplitSurface(tabId: tabId),
let sourceWorkspace = located.tabManager.tabs.first(where: { $0.id == located.workspaceId }),
let panel = sourceWorkspace.panels[located.panelId],
canTransferSurfaceAcrossWorkspaceBoundary(panel: panel) else {
return []
}
return workspaceMoveTargets(
excludingWorkspaceId: located.workspaceId,
referenceWindowId: located.windowId
Expand Down Expand Up @@ -88,7 +107,8 @@ extension AppDelegate {
guard let source = locateSurface(surfaceId: panelId),
let sourceWorkspace = source.tabManager.tabs.first(where: { $0.id == source.workspaceId }),
let sourcePanel = sourceWorkspace.panels[panelId],
sourceWorkspace.panels.count > 1 else {
sourceWorkspace.panels.count > 1,
canTransferSurfaceAcrossWorkspaceBoundary(panel: sourcePanel) else {
return nil
}

Expand Down
3 changes: 1 addition & 2 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13860,8 +13860,7 @@ struct TabItemView: View, Equatable {
// workspace-todos feature is on and there is either content or a
// pending "Add Checklist Item…" request (which needs the add
// field visible on an empty checklist).
if workspaceSnapshot.taskStatus != nil,
!workspaceSnapshot.checklistItems.isEmpty || checklistAddFieldActivationToken > 0 {
if !workspaceSnapshot.checklistItems.isEmpty || checklistAddFieldActivationToken > 0 {
SidebarWorkspaceChecklistSection(
items: workspaceSnapshot.checklistItems,
completedCount: workspaceSnapshot.checklistCompletedCount,
Expand Down
1 change: 1 addition & 0 deletions Sources/DockSplitStore+SurfaceTransfer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ extension DockSplitStore {
focus: Bool = true
) -> UUID? {
guard bonsplitController.allPaneIds.contains(paneId), panels[detached.panelId] == nil else { return nil }
guard detached.panel.panelType != .workspaceTodo else { return nil }
Comment on lines 210 to +211

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

Duplicated workspaceTodo eligibility check β€” consider centralizing.

This inline detached.panel.panelType != .workspaceTodo re-implements the same rule as AppDelegate.canTransferSurfaceAcrossWorkspaceBoundary(panel:) (Sources/AppDelegate+MoveTabToNewWorkspace.swift:15-17), and Workspace.attachDetachedSurface (Sources/Workspace.swift:9407-9409) has yet another inline copy with a same-workspace exception. Since DockSplitStore/Workspace can't call the AppDelegate extension method directly, the rule ends up duplicated three times. If the eligibility rule ever changes (e.g. another panel type becomes non-transferable), it's easy to update one site and miss the others.

Consider moving the base rule onto PanelType/Panel (e.g. panel.isTransferableAcrossWorkspaces) so all three call sites share one definition, with AppDelegate.canTransferSurfaceAcrossWorkspaceBoundary and Workspace's same-workspace exception both delegating to 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/DockSplitStore`+SurfaceTransfer.swift around lines 210 - 211, The
workspace-transfer eligibility rule for workspaceTodo is duplicated across
DockSplitStore, AppDelegate, and Workspace. Move the shared base check onto a
common symbol such as PanelType or Panel (for example a transferability
property/method), then update DockSplitStore+SurfaceTransfer,
AppDelegate.canTransferSurfaceAcrossWorkspaceBoundary(panel:), and
Workspace.attachDetachedSurface to delegate to that single definition, keeping
Workspace’s same-workspace exception separate if needed.

let panel = detached.panel

if let terminal = panel as? TerminalPanel {
Expand Down
17 changes: 8 additions & 9 deletions Sources/KeyboardShortcutSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -410,15 +410,11 @@ enum KeyboardShortcutSettings {
case .editWorkspaceDescription:
return StoredShortcut(key: "e", command: true, shift: false, option: true, control: false)
case .markWorkspaceDone:
// Cmd+; pins the selected workspace's status to done. The
// Cmd-"D" family was taken by split/diff actions and the
// natural Cmd+Ctrl+D chord is reserved by macOS; Cmd+;
// (semicolon) is free and sits next to the status-cycle chord.
return StoredShortcut(key: ";", command: true, shift: false, option: false, control: false)
// Ctrl+Cmd+; avoids macOS spelling shortcuts while keeping the status pair adjacent.
return StoredShortcut(key: ";", command: true, shift: false, option: false, control: true)
case .cycleWorkspaceStatus:
// Cmd+Shift+; cycles the selected workspace's status one lane
// forward (todo β†’ working β†’ needs-attention β†’ review β†’ done).
return StoredShortcut(key: ";", command: true, shift: true, option: false, control: false)
// Ctrl+Cmd+Shift+; cycles one lane forward without stealing spelling keys.
return StoredShortcut(key: ";", command: true, shift: true, option: false, control: true)
case .toggleChecklistItemComplete:
// Cmd+Return toggles the highlighted checklist item in the
// focused todo pane / checklist popover. Registered here for
Expand Down Expand Up @@ -633,7 +629,10 @@ enum KeyboardShortcutSettings {
}

var allowsChordShortcut: Bool {
self != .fileExplorerOpenSelection && self != .fileExplorerOpenSelectionFinderAlias && self != .cycleTextBoxSubmitAction
self != .fileExplorerOpenSelection
&& self != .fileExplorerOpenSelectionFinderAlias
&& self != .cycleTextBoxSubmitAction
&& self != .toggleChecklistItemComplete
}

var isBrowserContentShortcut: Bool {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Workspace+PanelLifecycle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ extension Workspace {

var agentLifecycleStatesByPanelId: [UUID: [String: AgentHibernationLifecycleState]] {
get { sidebarAgentRuntimeObservation.agentLifecycleStatesByPanelId }
set { sidebarAgentRuntimeObservation.setAgentLifecycleStatesByPanelId(newValue) }
set { sidebarAgentRuntimeObservation.setAgentLifecycleStatesByPanelId(newValue); reconcileExpiredTaskStatusOverride() }
}

func agentRuntimeState(forPanelId panelId: UUID) -> DetachedAgentRuntimeState? {
Expand Down
11 changes: 7 additions & 4 deletions Sources/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2275,19 +2275,19 @@ final class Workspace: Identifiable, ObservableObject {
}
var gitBranch: SidebarGitBranchState? {
get { sidebarMetadata.gitBranch }
set { sidebarMetadata.gitBranch = newValue }
set { sidebarMetadata.gitBranch = newValue; reconcileExpiredTaskStatusOverride() }
}
var panelGitBranches: [UUID: SidebarGitBranchState] {
get { sidebarMetadata.panelGitBranches }
set { sidebarMetadata.panelGitBranches = newValue }
set { sidebarMetadata.panelGitBranches = newValue; reconcileExpiredTaskStatusOverride() }
}
var pullRequest: SidebarPullRequestState? {
get { sidebarMetadata.pullRequest }
set { sidebarMetadata.pullRequest = newValue }
set { sidebarMetadata.pullRequest = newValue; reconcileExpiredTaskStatusOverride() }
}
var panelPullRequests: [UUID: SidebarPullRequestState] {
get { sidebarMetadata.panelPullRequests }
set { sidebarMetadata.panelPullRequests = newValue }
set { sidebarMetadata.panelPullRequests = newValue; reconcileExpiredTaskStatusOverride() }
}
@Published var surfaceListeningPorts: [UUID: [Int]] = [:]
var agentListeningPorts: [Int] = []
Expand Down Expand Up @@ -9404,6 +9404,9 @@ final class Workspace: Identifiable, ObservableObject {
#endif
return nil
}
guard detached.panel.panelType != .workspaceTodo || detached.sourceWorkspaceId == id else {
return nil
}

if let directory = detached.directory {
panelDirectories[detached.panelId] = directory
Expand Down
4 changes: 2 additions & 2 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ Default: `enabled: false`. The setting turns on automatically the first time a s

Three keyboard shortcuts drive the todo state, all editable in **Settings > Keyboard Shortcuts** or `shortcuts.bindings`:

- `markWorkspaceDone` (default `cmd+;`) pins the selected workspace's status to done.
- `cycleWorkspaceStatus` (default `cmd+shift+;`) advances the status one lane forward (todo β†’ working β†’ needs-attention β†’ review β†’ done β†’ todo).
- `markWorkspaceDone` (default `ctrl+cmd+;`) pins the selected workspace's status to done.
- `cycleWorkspaceStatus` (default `ctrl+cmd+shift+;`) advances the status one lane forward (todo β†’ working β†’ needs-attention β†’ review β†’ done β†’ todo).
- `toggleChecklistItemComplete` (default `cmd+return`) toggles the highlighted checklist item in the focused todo pane or checklist popover.

cmux also posts a notification when a workspace's status first reaches done, and when its checklist first becomes fully complete, so you can watch agent progress without keeping the pane open.
Loading
Loading