Restore workspaces-as-todos, default new workspaces to None status - #7790
Conversation
Reverts the revert (PR #7761) of "Workspaces as todos: inferred status lifecycle + per-workspace checklist" (#7216), restoring the feature. Additionally flips WorkspaceTodoState.statusHidden's default to true so newly created workspaces start with the status glyph hidden (the existing "None" opt-out state) instead of Auto — the feature is opt-in per workspace going forward. Pre-existing persisted workspaces that predate this field still restore to their historical visible/Auto state.
|
Too many files changed for review. ( Bypass the limit by tagging |
|
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:
📝 WalkthroughWalkthroughThis PR adds workspace status and todo checklists across the CLI, control socket, workspace model, persistence, sidebar, todo pane, keyboard shortcuts, settings, notifications, tests, and documentation. ChangesWorkspace todo feature
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ControlCommandCoordinator
participant TerminalController
participant Workspace
participant Sidebar
User->>CLI: run workspace status or todo command
CLI->>ControlCommandCoordinator: send workspace.todo request
ControlCommandCoordinator->>TerminalController: resolve workspace and mutate state
TerminalController->>Workspace: update status or checklist
Workspace-->>Sidebar: publish updated todo state
Sidebar-->>User: render status and checklist
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 errors, 2 warnings)
✅ Passed checks (18 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 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c357069. Configure here.
| "all_pull_requests_merged_or_closed": .bool(status.signals.allPullRequestsMergedOrClosed), | ||
| "is_git_dirty": .bool(status.signals.isGitDirty), | ||
| ]), | ||
| ]) |
There was a problem hiding this comment.
Status API omits hidden flag
Medium Severity
workspace.status.get and the CLI status output never expose statusHidden, so a workspace in None (glyph hidden) looks identical to visible Auto: override is null and effective/inferred reflect live signals. With new workspaces defaulting to hidden, scripts and agents can misread opt-out as engaged automatic status.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c357069. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 `@CLI/CMUXCLI`+WorkspaceTodo.swift:
- Around line 346-381: Update the static todoUsage help text to document the
supported move/mv subcommand, adding “move <index|id> <newIndex>” with the other
Subcommands entries and noting the mv alias if appropriate. Keep the syntax
consistent with the existing command handling and generic error hints.
- Around line 227-232: In the JSON parsing error handler within the todo-setting
logic, replace error.localizedDescription with String(describing: error) when
constructing the CLIError message, preserving the detailed underlying
serialization error.
In
`@Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator`+WorkspaceTodo.swift:
- Around line 143-144: Replace implementation-specific messages such as
“TabManager not available” and “Unexpected resolution” in the error responses
handled by the WorkspaceTodo control-command coordinator with stable,
product-facing messages, while preserving the existing error codes and keeping
diagnostic details internal. Apply this consistently to the error branches
around tab-manager unavailability and unexpected resolution, including the cases
identified near lines 175, 290, 314, 338, 358, 383, and 398.
- Around line 192-199: The workspaceTodoItemSelector function must reject
malformed selectors: if id or index is supplied but cannot be parsed, return nil
rather than falling back to another selector. Update set_state, edit, remove,
and move resolution failures to use product-facing error terminology instead of
“TabManager” or “Unexpected resolution.”
In
`@Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift`:
- Around line 382-387: The new shortcut labels in the `ShortcutAction` localized
strings require matching catalog entries. Add
`shortcut.markWorkspaceDone.label`, `shortcut.cycleWorkspaceStatus.label`, and
`shortcut.toggleChecklistItemComplete.label` to `Localizable.xcstrings` with
translations for every supported app locale, preserving the existing default
English values and localized API usage.
In
`@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklist.swift`:
- Around line 103-111: Clamp the externally supplied toIndex to a nonnegative
value before subtracting uncompleted.count in moveChecklistItem(id:toIndex:).
Update the completed-item local index calculation to avoid integer underflow for
Int.min, then continue clamping the result to the valid completed range.
- Around line 42-56: Update WorkspaceChecklist.addChecklistItem so newly added
pending items are inserted immediately before the first completed item rather
than always appended, while preserving the existing behavior for completed items
and checklist capacity validation. Add a regression test covering adding an
item, completing it, then adding a default pending item and asserting the
completed item remains last.
In `@Sources/Panels/WorkspaceTodoPanelView.swift`:
- Around line 145-152: The workspace header currently renders the
inferred-status glyph even when status is hidden. In the header view near
SidebarWorkspaceTaskStatusGlyph, conditionally render the glyph only when
!todoState.statusHidden, while preserving a separate accessible Status-menu path
so hidden statuses remain available through the menu.
In `@Sources/SidebarWorkspaceChecklistPopover.swift`:
- Around line 182-195: Update moreRow(hiddenCount:) to choose the localization
key sidebar.checklist.moreItems.one when hiddenCount equals 1 and
sidebar.checklist.moreItems.other otherwise, preserving the count substitution
for both variants. Add or update translations for both plural-specific keys and
remove use of the single sidebar.checklist.moreItems key.
In `@Sources/VerticalTabsSidebar`+EmptyAreasAndFooter.swift:
- Around line 69-138: Remove the `@EnvironmentObject` TabManager dependency from
SidebarEmptyArea. Move the selected-tab remote-mirror check, workspace creation,
selected-tab lookup, and related state updates from its onTapGesture into the
parent call sites in ContentView, then pass snapshot values and an action
closure into SidebarEmptyArea while preserving the existing double-tap behavior
and selection updates.
In `@Sources/Workspace`+TodoNotifications.swift:
- Around line 4-16: The edge-triggered notification logic must not suppress
valid repeated transitions. Remove the shared short cooldown key and all related
timing/deduplication checks from the workspace todo notification wrappers,
including the logic around the effective `.done` and checklist-complete
crossings; rely solely on the before/after state comparisons in the `notifying…`
mutation wrappers to prevent duplicate notifications.
In `@Sources/WorkspaceTodoFeature.swift`:
- Around line 12-32: Replace the static-only WorkspaceTodoFeature and
WorkspaceTodoActions APIs with a constructable `@MainActor` coordinator that owns
checklist policy, persistence access, and mutation routing. Move isEnabled,
checklistStyle, markUsed, and action behavior onto that coordinator while
preserving one shared action path. Inject the coordinator into UI, shortcut, and
socket adapters, and update their call sites to use the instance rather than
static methods or ambient global state.
In `@Sources/WorkspaceTodoState.swift`:
- Around line 1-25: Consider migrating WorkspaceTodoState from
ObservableObject/@Published to the repo’s `@Observable` pattern, updating all
current Combine-based consumers and observation access accordingly; otherwise
leave this implementation unchanged.
In `@web/data/cmux.schema.json`:
- Around line 978-997: Add stable descriptionKey fields to the beta,
workspaceTodos, and checklistStyle schema entries, then add matching English and
Japanese translations in the schema-description catalogs web/messages/en.json
and web/messages/ja.json. Keep the keys consistent across schema and both
catalogs, and preserve fallback behavior for other locales.
🪄 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: 4a117bb8-1538-408c-bba1-af45be0e0b74
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (103)
CLI/CMUXCLI+CommandSuggestions.swiftCLI/CMUXCLI+WorkspaceTodo.swiftCLI/cmux.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/ControlCommandContext.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/ControlCommandCoordinator.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlCommandCoordinator+Debug.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlDebugContext.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator+WorkspaceTodo.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator+WorkspaceTodoSetOpen.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoChecklistResolution.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoChecklistSnapshot.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoContext.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoMutationResolution.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoOpenResolution.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoSetResolution.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoStatusResolution.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlWorkspaceTodoStatusSnapshot.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs+Debug.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceTodoSetOpenTests.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorWorkspaceTodoTests.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlWorkspaceTodoContextTestStubs.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/WorkspaceTodoChecklistStyle.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BetaFeaturesSection.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Core/Values/SurfaceKind.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklist.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistItem.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistProgressSummary.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceChecklistReplacement.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceTaskStatus.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceTaskStatusOverride.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceTaskStatusSignals.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Values/WorkspaceChecklistReplacementTests.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Values/WorkspaceChecklistTests.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Values/WorkspaceTaskStatusTests.swiftResources/Localizable.xcstringsSources/AllShortcutsPopover.swiftSources/App/ForeignFirstResponderPolicy.swiftSources/AppDelegate+WorkspaceTodoShortcut.swiftSources/AppDelegate.swiftSources/Canvas/WorkspaceCanvasHostView.swiftSources/ChecklistInputField.swiftSources/ClosedItemHistory.swiftSources/CmuxLifecycleEventPublishing.swiftSources/CmuxSettingsJSONPathSupport.swiftSources/ContentView+RightSidebarCommandPalette.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/KeyboardShortcutSettingsFileStore+SectionParsers.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Mobile/MobileWorkspaceListObserver.swiftSources/Panels/Panel.swiftSources/Panels/PanelContentView.swiftSources/Panels/WorkspaceTodoPanel.swiftSources/Panels/WorkspaceTodoPanelView.swiftSources/Search/GlobalSearchDocuments.swiftSources/SessionPersistence+RightSidebarTool.swiftSources/SessionPersistence+Todos.swiftSources/SessionPersistence.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/ShortcutDiscoveryButton.swiftSources/Sidebar/SidebarWorkspaceSnapshotRefreshPolicy.swiftSources/SidebarTabItemContextMenuState.swiftSources/SidebarWorkspaceChecklistPopover.swiftSources/SidebarWorkspaceChecklistView.swiftSources/SidebarWorkspaceSnapshotBuilder.swiftSources/SidebarWorkspaceStatusPopover.swiftSources/SidebarWorkspaceTaskStatusGlyph.swiftSources/SidebarWorkspaceTodoPopoverHost.swiftSources/TabItemView+WorkspaceTodo.swiftSources/TabManager.swiftSources/TerminalController+ControlDebugContext.swiftSources/TerminalController+ControlWorkspaceTodoContext.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController.swiftSources/TerminalPaneDropTargetView.swiftSources/VerticalTabsSidebar+EmptyAreasAndFooter.swiftSources/Workspace+LayoutCapture.swiftSources/Workspace+SurfaceNavigation.swiftSources/Workspace+TodoNotifications.swiftSources/Workspace+TodoPane.swiftSources/Workspace+Todos.swiftSources/Workspace.swiftSources/WorkspaceSidebarObservation.swiftSources/WorkspaceTodoFeature.swiftSources/WorkspaceTodoState.swiftcmux.xcodeproj/project.pbxprojcmuxTests/ForeignFirstResponderPolicyTests.swiftcmuxTests/SidebarWorkspaceRowInteractionStateTests.swiftcmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swiftcmuxTests/WorkspaceTodoSidebarModelTests.swiftcmuxTests/WorkspaceTodoSnapshotTests.swiftdocs/cli-contract.mddocs/configuration.mdscripts/swift_file_length_budget.pyweb/data/cmux-shortcuts.tsweb/data/cmux.schema.json
💤 Files with no reviewable changes (1)
- Sources/SessionPersistence+RightSidebarTool.swift
| let parsed: Any | ||
| do { | ||
| parsed = try JSONSerialization.jsonObject(with: Data(trimmed.utf8)) | ||
| } catch { | ||
| throw CLIError(message: "Invalid JSON for todo set: \(error.localizedDescription)") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prefer String(describing: error) over error.localizedDescription for CLI error formatting.
JSONSerialization's thrown NSError may report a generic Cocoa message via localizedDescription, obscuring the real cause of the malformed JSON.
Based on learnings: "Use String(describing: error) instead of error.localizedDescription when formatting errors in the cmux Swift CLI (CLI/**/*.swift). Non-LocalizedError types like CocoaError/NSError can reveal a generic message when using localizedDescription, which hides the real cause."
🩹 Proposed fix
- throw CLIError(message: "Invalid JSON for todo set: \(error.localizedDescription)")
+ throw CLIError(message: "Invalid JSON for todo set: \(String(describing: error))")📝 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.
| let parsed: Any | |
| do { | |
| parsed = try JSONSerialization.jsonObject(with: Data(trimmed.utf8)) | |
| } catch { | |
| throw CLIError(message: "Invalid JSON for todo set: \(error.localizedDescription)") | |
| } | |
| let parsed: Any | |
| do { | |
| parsed = try JSONSerialization.jsonObject(with: Data(trimmed.utf8)) | |
| } catch { | |
| throw CLIError(message: "Invalid JSON for todo set: \(String(describing: error))") | |
| } |
🤖 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 `@CLI/CMUXCLI`+WorkspaceTodo.swift around lines 227 - 232, In the JSON parsing
error handler within the todo-setting logic, replace error.localizedDescription
with String(describing: error) when constructing the CLIError message,
preserving the detailed underlying serialization error.
Source: Learnings
| static let todoUsage = String(localized: "cli.todo.usage", defaultValue: """ | ||
| Usage: cmux todo <subcommand> [--workspace <id|ref|index>] [--window <id|ref|index>] [--json] | ||
|
|
||
| Per-workspace checklist, writable by you and by agents. Targets the | ||
| caller's workspace by default. Items are capped at 50 per workspace. | ||
|
|
||
| Subcommands: | ||
| add "text" [--state <pending|in-progress|completed>] [--origin <user|agent>] | ||
| list Print items (1-based indexes) and progress | ||
| check <index|id> Mark an item completed | ||
| uncheck <index|id> Mark an item pending | ||
| start <index|id> Mark an item in-progress | ||
| edit <index|id> "text" Rewrite an item's text | ||
| rm <index|id> Remove an item | ||
| clear Remove every item | ||
| set ['<json>'] Atomically replace the whole checklist from a | ||
| JSON array of {text, state?, id?, origin?} | ||
| objects (inline argument, or piped on stdin). | ||
| Items whose id matches an existing item keep | ||
| their identity and origin; the rest are created | ||
| and unnamed existing items are removed. | ||
| open Open (or focus) the workspace's todo pane | ||
|
|
||
| <index> is the 1-based number printed by `cmux todo list`; <id> is the | ||
| item UUID from `cmux todo list --json`. | ||
|
|
||
| Examples: | ||
| cmux todo add "write regression test" | ||
| cmux todo list | ||
| cmux todo check 1 | ||
| cmux todo start 2 --workspace workspace:3 | ||
| my-plan-tool --json | cmux todo set | ||
| cmux todo open | ||
|
|
||
| See also: cmux workspace status | ||
| """) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
todoUsage is missing the move/mv subcommand.
The switch statement (Lines 176-187) and the generic error hints (Lines 121, 200) both support/list move <index|id> <newIndex>, but the printed help text never documents its syntax, leaving users unaware of how to reorder items via cmux todo --help.
📝 Proposed fix
rm <index|id> Remove an item
+ move <index|id> <newIndex>
+ Reorder an item (newIndex is 1-based)
clear Remove every item📝 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.
| static let todoUsage = String(localized: "cli.todo.usage", defaultValue: """ | |
| Usage: cmux todo <subcommand> [--workspace <id|ref|index>] [--window <id|ref|index>] [--json] | |
| Per-workspace checklist, writable by you and by agents. Targets the | |
| caller's workspace by default. Items are capped at 50 per workspace. | |
| Subcommands: | |
| add "text" [--state <pending|in-progress|completed>] [--origin <user|agent>] | |
| list Print items (1-based indexes) and progress | |
| check <index|id> Mark an item completed | |
| uncheck <index|id> Mark an item pending | |
| start <index|id> Mark an item in-progress | |
| edit <index|id> "text" Rewrite an item's text | |
| rm <index|id> Remove an item | |
| clear Remove every item | |
| set ['<json>'] Atomically replace the whole checklist from a | |
| JSON array of {text, state?, id?, origin?} | |
| objects (inline argument, or piped on stdin). | |
| Items whose id matches an existing item keep | |
| their identity and origin; the rest are created | |
| and unnamed existing items are removed. | |
| open Open (or focus) the workspace's todo pane | |
| <index> is the 1-based number printed by `cmux todo list`; <id> is the | |
| item UUID from `cmux todo list --json`. | |
| Examples: | |
| cmux todo add "write regression test" | |
| cmux todo list | |
| cmux todo check 1 | |
| cmux todo start 2 --workspace workspace:3 | |
| my-plan-tool --json | cmux todo set | |
| cmux todo open | |
| See also: cmux workspace status | |
| """) | |
| static let todoUsage = String(localized: "cli.todo.usage", defaultValue: """ | |
| Usage: cmux todo <subcommand> [--workspace <id|ref|index>] [--window <id|ref|index>] [--json] | |
| Per-workspace checklist, writable by you and by agents. Targets the | |
| caller's workspace by default. Items are capped at 50 per workspace. | |
| Subcommands: | |
| add "text" [--state <pending|in-progress|completed>] [--origin <user|agent>] | |
| list Print items (1-based indexes) and progress | |
| check <index|id> Mark an item completed | |
| uncheck <index|id> Mark an item pending | |
| start <index|id> Mark an item in-progress | |
| edit <index|id> "text" Rewrite an item's text | |
| rm <index|id> Remove an item | |
| move <index|id> <newIndex> | |
| Reorder an item (newIndex is 1-based) | |
| clear Remove every item | |
| set ['<json>'] Atomically replace the whole checklist from a | |
| JSON array of {text, state?, id?, origin?} | |
| objects (inline argument, or piped on stdin). | |
| Items whose id matches an existing item keep | |
| their identity and origin; the rest are created | |
| and unnamed existing items are removed. | |
| open Open (or focus) the workspace's todo pane | |
| <index> is the 1-based number printed by `cmux todo list`; <id> is the | |
| item UUID from `cmux todo list --json`. | |
| Examples: | |
| cmux todo add "write regression test" | |
| cmux todo list | |
| cmux todo check 1 | |
| cmux todo start 2 --workspace workspace:3 | |
| my-plan-tool --json | cmux todo set | |
| cmux todo open | |
| See also: cmux workspace status | |
| """) |
🤖 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 `@CLI/CMUXCLI`+WorkspaceTodo.swift around lines 346 - 381, Update the static
todoUsage help text to document the supported move/mv subcommand, adding “move
<index|id> <newIndex>” with the other Subcommands entries and noting the mv
alias if appropriate. Keep the syntax consistent with the existing command
handling and generic error hints.
| case .tabManagerUnavailable: | ||
| return .err(code: "unavailable", message: "TabManager not available", data: nil) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Keep control-socket errors free of implementation details.
"TabManager not available" and "Unexpected resolution" expose internal type/control-flow details in API error bodies. Return stable product-facing messages while retaining the existing error codes; keep implementation diagnostics internal.
As per coding guidelines, user-facing API error bodies must use product terms and must not expose implementation details.
Also applies to: 175-176, 290-291, 314-315, 338-339, 358-359, 383-384, 398-399
🤖 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/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator`+WorkspaceTodo.swift
around lines 143 - 144, Replace implementation-specific messages such as
“TabManager not available” and “Unexpected resolution” in the error responses
handled by the WorkspaceTodo control-command coordinator with stable,
product-facing messages, while preserving the existing error codes and keeping
diagnostic details internal. Apply this consistently to the error branches
around tab-manager unavailability and unexpected resolution, including the cases
identified near lines 175, 290, 314, 338, 358, 383, and 398.
Source: Coding guidelines
| private func workspaceTodoItemSelector( | ||
| _ params: [String: JSONValue] | ||
| ) -> (itemID: UUID?, itemIndex: Int?)? { | ||
| let itemID = uuid(params, "id") | ||
| let itemIndex = int(params, "index") | ||
| guard itemID != nil || itemIndex != nil else { return nil } | ||
| return (itemID, itemIndex) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject malformed selectors instead of falling back to index. If id is present but invalid, workspaceTodoItemSelector returns (nil, index), so set_state / edit / remove / move can hit a different item than the caller targeted. Fail closed on any supplied invalid selector, and replace the TabManager / Unexpected resolution error text with product terms.
🤖 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/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceTodo/ControlCommandCoordinator`+WorkspaceTodo.swift
around lines 192 - 199, The workspaceTodoItemSelector function must reject
malformed selectors: if id or index is supplied but cannot be parsed, return nil
rather than falling back to another selector. Update set_state, edit, remove,
and move resolution failures to use product-facing error terminology instead of
“TabManager” or “Unexpected resolution.”
| case .markWorkspaceDone: | ||
| return String(localized: "shortcut.markWorkspaceDone.label", defaultValue: "Mark Workspace as Done") | ||
| case .cycleWorkspaceStatus: | ||
| return String(localized: "shortcut.cycleWorkspaceStatus.label", defaultValue: "Cycle Workspace Status") | ||
| case .toggleChecklistItemComplete: | ||
| return String(localized: "shortcut.toggleChecklistItemComplete.label", defaultValue: "Toggle Checklist Item Complete") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the new shortcut-label translations.
These new keys have no matching Resources/Localizable.xcstrings change in the reviewed stack, so non-English settings UI falls back to English. Add translated entries for all supported app locales.
As per coding guidelines, user-facing Swift text requires localized APIs with matching catalog entries.
🤖 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/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift`
around lines 382 - 387, The new shortcut labels in the `ShortcutAction`
localized strings require matching catalog entries. Add
`shortcut.markWorkspaceDone.label`, `shortcut.cycleWorkspaceStatus.label`, and
`shortcut.toggleChecklistItemComplete.label` to `Localizable.xcstrings` with
translations for every supported app locale, preserving the existing default
English values and localized API usage.
Sources: Coding guidelines, Path instructions
| struct SidebarEmptyArea: View { | ||
| @EnvironmentObject var tabManager: TabManager | ||
| let rowSpacing: CGFloat | ||
| @Binding var selection: SidebarSelection | ||
| @Binding var selectedTabIds: Set<UUID> | ||
| @Binding var lastSidebarSelectionIndex: Int? | ||
| let dragAutoScrollController: SidebarDragAutoScrollController | ||
| // Value snapshot + closure bundles instead of an @Observable store | ||
| // reference (snapshot-boundary rule). | ||
| let topDropIndicatorVisible: Bool | ||
| var tabDropDelegate: SidebarTabDropDelegate? = nil | ||
| let bonsplitDropIndicator: Binding<SidebarDropIndicator?> | ||
| var expandsVertically = true | ||
| var minimumHeight: CGFloat? = nil | ||
|
|
||
| var body: some View { | ||
| dropTarget | ||
| .overlay { | ||
| SidebarBonsplitTabNewWorkspaceDropOverlay( | ||
| tabManager: tabManager, | ||
| selectedTabIds: $selectedTabIds, | ||
| lastSidebarSelectionIndex: $lastSidebarSelectionIndex, | ||
| dropIndicator: bonsplitDropIndicator | ||
| ) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| } | ||
| .overlay(alignment: .top) { | ||
| if topDropIndicatorVisible { | ||
| Rectangle() | ||
| .fill(cmuxAccentColor()) | ||
| .frame(height: 2) | ||
| .padding(.horizontal, 8) | ||
| .offset(y: -(rowSpacing / 2)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @ViewBuilder | ||
| private var dropTarget: some View { | ||
| let base = hitTarget | ||
| .onTapGesture(count: 2) { | ||
| // When the active workspace is a remote-tmux mirror, route through | ||
| // performNewWorkspaceAction so a new workspace becomes a new tmux | ||
| // session instead of a local (orphan) workspace. Gate on the | ||
| // SELECTED tab, not `tabs.contains`: a dedicated remote window can | ||
| // be polluted with a dragged-in local workspace (move targets don't | ||
| // exclude dedicated windows), and `contains` would then misroute a | ||
| // local empty-area double-tap into spawning an unwanted tmux session. | ||
| if tabManager.selectedTab?.isRemoteTmuxMirror == true { | ||
| _ = AppDelegate.shared?.performNewWorkspaceAction( | ||
| tabManager: tabManager, | ||
| debugSource: "sidebar.emptyArea.remoteTmux" | ||
| ) | ||
| } else { | ||
| tabManager.addWorkspace(placementOverride: .end) | ||
| } | ||
| if let selectedId = tabManager.selectedTabId { | ||
| selectedTabIds = [selectedId] | ||
| lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == selectedId } | ||
| } | ||
| selection = .tabs | ||
| } | ||
| if let tabDropDelegate { | ||
| base | ||
| .sidebarEmptyAreaWorkspaceGroupContextMenu(tabManager: tabManager) | ||
| .onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: tabDropDelegate) | ||
| } else { | ||
| base | ||
| .sidebarEmptyAreaWorkspaceGroupContextMenu(tabManager: tabManager) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where SidebarEmptyArea is instantiated and inspect surrounding structure
rg -n 'SidebarEmptyArea\(' --type=swift -C 15Repository: manaflow-ai/cmux
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all references to the type and inspect the sidebar container structure.
rg -n --type=swift '\bSidebarEmptyArea\b' .
echo '---'
fd -a 'VerticalTabsSidebar+EmptyAreasAndFooter.swift' .
echo '---'
fd -a 'VerticalTabsSidebar*.swift' SourcesRepository: manaflow-ai/cmux
Length of output: 807
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the call sites and surrounding sidebar container structure.
sed -n '10790,10880p' Sources/ContentView.swift
echo '---'
sed -n '11810,11920p' Sources/ContentView.swift
echo '---'
sed -n '10150,10210p' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 13248
Remove the live TabManager dependency from SidebarEmptyArea
The current call sites in Sources/ContentView.swift still mount this view under the sidebar row subtree, so @EnvironmentObject var tabManager keeps a live store reference below the LazyVStack/ForEach boundary. Move the remaining tabManager reads/actions into the parent and pass snapshots/closures into SidebarEmptyArea instead.
🤖 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/VerticalTabsSidebar`+EmptyAreasAndFooter.swift around lines 69 - 138,
Remove the `@EnvironmentObject` TabManager dependency from SidebarEmptyArea. Move
the selected-tab remote-mirror check, workspace creation, selected-tab lookup,
and related state updates from its onTapGesture into the parent call sites in
ContentView, then pass snapshot values and an action closure into
SidebarEmptyArea while preserving the existing double-tap behavior and selection
updates.
Sources: Coding guidelines, Learnings
| /// Transition notifications for a workspace's todo state: a single cmux | ||
| /// notification when the effective status first reaches `.done`, and one when | ||
| /// the checklist first becomes fully complete (n/n, n > 0). Both reuse the | ||
| /// same delivery path as the app's other workspace-level notifications | ||
| /// (`AppDelegate.shared.notificationStore.addNotification`, `surfaceId: nil`), | ||
| /// mirroring `Workspace.applyRemoteConnectionStateUpdate`. | ||
| /// | ||
| /// The `Workspace+Todos` mutation entry points route their mutation through | ||
| /// the `notifying…` wrappers below, which sample the before/after state and | ||
| /// fire only on the crossing edge, so the notification is emitted once per | ||
| /// transition regardless of whether the user, the CLI, or an agent (socket) | ||
| /// drove the change. A short cooldown key guards against a duplicate landing | ||
| /// from two entry points in the same tick. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not time-suppress distinct completion transitions.
A workspace can move done → non-done → done (or checklist complete → incomplete → complete) within three seconds. The shared cooldown key suppresses the second valid crossing, despite the before/after state check already being the authoritative deduplication mechanism. Remove the cooldown for these edge-triggered notifications.
Also applies to: 61-62, 79-86
🤖 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/Workspace`+TodoNotifications.swift around lines 4 - 16, The
edge-triggered notification logic must not suppress valid repeated transitions.
Remove the shared short cooldown key and all related timing/deduplication checks
from the workspace todo notification wrappers, including the logic around the
effective `.done` and checklist-complete crossings; rely solely on the
before/after state comparisons in the `notifying…` mutation wrappers to prevent
duplicate notifications.
| enum WorkspaceTodoFeature { | ||
| /// Synchronous read of the feature flag for on-demand paths. Reads only | ||
| /// the beta catalog section, not the whole `SettingCatalog`, so a | ||
| /// body-path access stays cheap (see issue #5970); reactive row reads go | ||
| /// through `SidebarTabItemSettingsSnapshot`. | ||
| /// The workspace-todos feature is always on (accessed via the row context | ||
| /// menu and status glyph); the Settings feature-flag toggle was removed. | ||
| static var isEnabled: Bool { true } | ||
|
|
||
| /// The checklist presentation style (popover or inline), user-selectable. | ||
| static var checklistStyle: WorkspaceTodoChecklistStyle { | ||
| let key = BetaFeaturesCatalogSection().workspaceTodosChecklistStyle | ||
| return WorkspaceTodoChecklistStyle.decodeFromUserDefaults( | ||
| UserDefaults.standard.object(forKey: key.userDefaultsKey) | ||
| ) ?? key.defaultValue | ||
| } | ||
|
|
||
| /// No-op now that the feature is always on (kept so existing call sites | ||
| /// stay unchanged). | ||
| @MainActor | ||
| static func markUsed() {} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Move todo policy and mutations behind an injectable owner.
WorkspaceTodoFeature and WorkspaceTodoActions introduce static-only production namespaces for feature policy, persistence access, and mutation routing. Keep one shared action path, but put it on a constructable @MainActor coordinator and inject that owner into the UI, shortcut, and socket adapters rather than calling static methods directly.
As per coding guidelines, production Swift must avoid ambient global state and static-only namespaces; behavior and state should live on a constructable, injectable owning type.
Also applies to: 39-153
🤖 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/WorkspaceTodoFeature.swift` around lines 12 - 32, Replace the
static-only WorkspaceTodoFeature and WorkspaceTodoActions APIs with a
constructable `@MainActor` coordinator that owns checklist policy, persistence
access, and mutation routing. Move isEnabled, checklistStyle, markUsed, and
action behavior onto that coordinator while preserving one shared action path.
Inject the coordinator into UI, shortcut, and socket adapters, and update their
call sites to use the instance rather than static methods or ambient global
state.
Source: Coding guidelines
| import Combine | ||
| import CmuxWorkspaces | ||
| import Foundation | ||
|
|
||
| /// The workspace-owned todo state: the manual task-status override and the | ||
| /// persisted checklist. A separate `ObservableObject` (held as a `let` on | ||
| /// `Workspace`, like `sidebarAgentRuntimeObservation`) so todo churn publishes | ||
| /// through its own object and sidebar rows can observe it without | ||
| /// invalidating on unrelated `Workspace` `@Published` traffic. | ||
| /// | ||
| /// All mutation goes through the `Workspace` entry points in | ||
| /// `Workspace+Todos.swift` (shared by socket verbs, CLI, and UI) so caps, | ||
| /// text normalization, and override anti-rot apply identically everywhere. | ||
| @MainActor | ||
| final class WorkspaceTodoState: ObservableObject { | ||
| /// The manual status override, or `nil` when the status is automatic. | ||
| /// Carries the inference recorded at override time so a stale override | ||
| /// auto-expires (see `WorkspaceTaskStatusOverride.effectiveStatus`). | ||
| @Published var statusOverride: WorkspaceTaskStatusOverride? | ||
| /// When true, this workspace opts out of the status feature: no glyph is | ||
| /// drawn before the title (a "None" state, distinct from Auto which still | ||
| /// infers and shows a glyph). Selecting Auto or any lane clears it. | ||
| @Published var statusHidden: Bool = true | ||
| /// The persisted checklist, in display order. | ||
| @Published var checklist: [WorkspaceChecklistItem] = [] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '--- WorkspaceTodoState.swift ---\n'
cat -n Sources/WorkspaceTodoState.swift
printf '\n--- Usages ---\n'
rg -n "WorkspaceTodoState|statusOverride|statusHidden|checklist" Sources cmuxTests cmuxUITests --glob '!**/build/**' --glob '!**/.build/**'Repository: manaflow-ai/cmux
Length of output: 245
🏁 Script executed:
set -euo pipefail
printf -- '--- WorkspaceTodoState.swift ---\n'
cat -n Sources/WorkspaceTodoState.swift
printf -- '\n--- Usages ---\n'
rg -n "WorkspaceTodoState|statusOverride|statusHidden|checklist" Sources cmuxTests cmuxUITests --glob '!**/build/**' --glob '!**/.build/**'Repository: manaflow-ai/cmux
Length of output: 32931
Prefer @Observable for the todo store
WorkspaceTodoState is a tiny main-actor state object, so ObservableObject/@Published adds extra Combine surface. If you want this state to follow the repo’s Observation pattern, migrate the store together with its current Combine consumers; otherwise leave it unchanged for now.
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 15-15: Classes should have an explicit deinit method
(required_deinit)
🤖 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/WorkspaceTodoState.swift` around lines 1 - 25, Consider migrating
WorkspaceTodoState from ObservableObject/@Published to the repo’s `@Observable`
pattern, updating all current Combine-based consumers and observation access
accordingly; otherwise leave this implementation unchanged.
Source: Coding guidelines
| "beta": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "description": "Experimental sidebar features.", | ||
| "properties": { | ||
| "workspaceTodos": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "description": "Workspace todos checklist presentation.", | ||
| "properties": { | ||
| "checklistStyle": { | ||
| "type": "string", | ||
| "enum": ["popover", "inline"], | ||
| "default": "popover", | ||
| "description": "How a row's checklist opens from its summary line: an anchored popover or inline expansion." | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Localize the new schema documentation.
The new descriptions are English-only literals without descriptionKeys, and the reviewed stack has no matching web/messages/en.json or web/messages/ja.json entries. Add stable schema-description keys and translations for the beta, workspace-todos, and checklist-style copy.
Based on learnings, schema-description catalogs are maintained in web/messages/en.json and web/messages/ja.json; other locales fall back through deepMergeMessages.
🤖 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 `@web/data/cmux.schema.json` around lines 978 - 997, Add stable descriptionKey
fields to the beta, workspaceTodos, and checklistStyle schema entries, then add
matching English and Japanese translations in the schema-description catalogs
web/messages/en.json and web/messages/ja.json. Keep the keys consistent across
schema and both catalogs, and preserve fallback behavior for other locales.
Sources: Coding guidelines, Path instructions, Learnings
…#7797) * Route the checklist's first item through the popover in popover style usesPopoverPresentation && totalCount > 0 forced inline entry for a workspace's very first checklist item regardless of the popover setting, since an empty checklist has no summary line to anchor a popover to. Move the popover anchor from the summary line to the section container (stable across the 0->1 item transition), route the ghost "Add item" button through the same popover-toggle path as the summary line, and fix the "Add Checklist Item..." notification handler in ContentView.swift (it special-cased non-empty checklists, so the context-menu/palette path still fell back to inline for a fresh workspace even after the view-level fix). Inline style is unaffected. * Fix: checklist section never rendered for hidden-status workspaces taskStatus is nil whenever a workspace's status is hidden (statusHidden), which restore-workspace-todos-default-none made the default for all new workspaces. The checklist section's render gate required taskStatus != nil, so "Add Checklist Item..." silently did nothing for any new workspace. Drop that condition; the checklist section only needs its own content/token gate. * Add hover-reveal delete button to checklist item rows The only way to delete a checklist item was a hidden right-click "Remove" context menu entry. Add a trailing xmark.circle.fill button that appears on row hover (reserved layout space, opacity/hit-test toggle only, so row height never jumps) in both row implementations: the inline sidebar row and the NSPopover-hosted row. Both call the existing actions.removeItem(_:) closure, the same one the context menu already used; the context menu entry is unchanged. New localized tooltip key sidebar.checklist.removeItemTooltip ("Remove item" / "項目を削除"). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix checklist popover focus ambiguity, ghost row, and item alignment; add keyboard delete - Highlighting a checklist item (click, Up/Down) and typing a new item in the add field are now mutually exclusive: a non-empty draft clears any highlight, and highlight-setting paths no-op while a draft exists. Makes Return's outcome (toggle item vs. commit new item) unambiguous. - Popover-style checklists no longer show a persistent ghost "Add item" row inline when a workspace has zero items. "Add Checklist Item…" now opens the popover directly with nothing shown inline until an item is actually added, at which point the existing summary line is the small status preview. - Item rows now center-align the checkbox and text (was firstTextBaseline, which misaligned the checkbox glyph against the text's baseline). - Backspace with an empty draft and a highlighted item now removes it, a keyboard-driven delete alongside the existing hover "x" and context menu "Remove". * Fix checklist popover anchor for zero-item workspaces An empty VStack collapses to a degenerate frame, which broke the NSPopover anchor view's bounds/window attachment when a workspace had zero checklist items — "Add Checklist Item…" silently failed to open the popover. Keep a minimal invisible placeholder so the anchor always has real geometry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix checklist popover open reliability, hover, scroll cap, and multi-line alignment The prior zero-item placeholder fix didn't retry reliably: it depended on an unrelated later re-render landing after the anchor NSView actually attached to its window, so "Add Checklist Item..." on a fresh workspace usually opened nothing until some other UI event fired. Replace it with an AnchorView subclass that retries present() from viewDidMoveToWindow(), so the popover opens deterministically as soon as AppKit finishes attaching it. The popover host's NSViewRepresentable anchor was also stacked as a .background() spanning the whole checklist section, which suppressed .onHover for the item rows underneath it (the hover-reveal delete "x" rarely appeared). Shrink the anchor to a fixed 1x1pt .overlay corner so it never intercepts hover/hit-testing, and drop the now-unnecessary Color.clear placeholder that caused the "bottom padding" artifact. Both the inline expansion and the popover now show all items in a ScrollView capped at 6 rows instead of hard-clamping with a "... N more" row, and item rows use .firstTextBaseline alignment with an alignmentGuide offset (derived from font ascender/descender) so the checkbox and delete button center on the first line of wrapped multi-line item text rather than the baseline or the whole block. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix checklist item text wrap in the dedicated Todo Pane view WorkspaceTodoPaneItemRow was missing the same multi-line-wrap fix already applied to the sidebar's inline list and popover: long item text overflowed unwrapped past the pane's edge instead of wrapping. Adds .fixedSize(horizontal: false, vertical: true) + .multilineTextAlignment(.leading) to the item Text, and centers the checkbox on the text's first line via .alignmentGuide(.firstTextBaseline). * Fix checklist popover zero-item anchor, hover-reveal delete, and re-present churn Three root causes, each verified against a live tagged build: - Zero-item anchor: the section VStack renders no children for an empty checklist in popover style, so .overlay(.topTrailing) collapsed to the row's LEFT edge. The container now claims the row's full width (maxWidth: .infinity) so the anchor always sits at the real right edge. - Hover-reveal delete x: per-row .onContinuousHover state died whenever popover content was recreated or rows reflowed under a stationary pointer (no new mouse-moved event arrives). hoveredItemId is now DERIVED from one container-level pointer location plus row frames collected via preference, seeded from NSEvent.mouseLocation at window attach, so the x survives model changes and fresh presents. - Re-present churn: when AppKit closed the transient popover (app deactivation, click-away) the container's isPresented=false write landed asynchronously, so a stale re-render tick re-presented the popover the user just dismissed (observed live: five didShows in 18s). The host coordinator now latches after an AppKit-side close until the container acknowledges false; an explicit add-request token change clears the latch so "Add Checklist Item…" can always present. Also: programmatic/keyboard workspace switches now dismiss open checklist/status popovers (no outside click for transient behavior to catch), and the Todo Pane keeps arrow-key scroll-follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Own checklist popover hover tracking in AppKit, not SwiftUI hover modifiers Independent artifact verification of the previous commit caught the delete x vanishing on a 1px pointer move right after a checklist mutation: SwiftUI's .onContinuousHover rebuilds its NSTrackingArea on content updates, and the first mouse event after a rebuild can arrive as a spurious .ended from the torn-down area with no follow-up .active until the next event. The popover's pointer location is now tracked by a persistent AppKit NSView (PopoverPointerTracker) whose NSTrackingArea only changes with geometry, never content: mouseEntered/mouseMoved report the location, mouseExited clears it, and viewDidMoveToWindow seeds from NSEvent.mouseLocation so a popover presenting under a resting pointer still shows hover affordances. .activeAlways keeps hover alive when the terminal pane steals key status from the popover window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Split checklist popover pointer tracking and summary modifier into own files Keeps SidebarWorkspaceChecklistPopover.swift and SidebarWorkspaceChecklistView.swift under the 500-line tracking threshold of the Swift file-length budget gate. Pure moves: PopoverPointerTracker + ChecklistPopoverRowFramesKey to ChecklistPopoverPointerTracking.swift, ChecklistSummaryPopoverModifier to ChecklistSummaryPopoverModifier.swift (private -> internal). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Ratchet ContentView.swift file-length budget down to actual (16262) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Trim ContentView comment growth to net zero for the file-length hard cap Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix spontaneous checklist popover re-presents and Open as Pane focus Root cause of the popover opening by itself while typing in the todo pane (user video 2026-07-13): the popover host coordinator captured its isPresented @binding once at creation. The binding's get closure closes over that render's value snapshot, so a coordinator created while the popover was hidden read a frozen `false` forever. On an AppKit-side close (click-away), popoverDidClose's `if isPresented` guard read that stale false and skipped writing false back, leaving the container's checklistPopoverWorkspaceId stuck set — and every later model change (e.g. committing an item in the todo pane) re-rendered the host with isPresented=true and re-presented the popover with no user action. The coordinator now holds a Binding<Bool> refreshed on every updateNSView tick, so reads and write-backs always target current container state. Reproduced deterministically before the fix (todo add to the selected workspace -> didShow ~1s later, zero user actions). Open as Pane now focuses an already-open todo pane usefully: the footer closes the popover BEFORE opening the pane (NSPopover teardown restores the previous first responder, which clobbered the focus openPane had just set), and openOrFocusWorkspaceTodoSurface bumps a new addFieldArmToken on WorkspaceTodoPanel so the pane re-arms its add field even when it was already focused and isFocused never transitions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Only nonzero activation tokens unlatch the popover's dismissal ack The external-dismissal latch treated ANY presentationRequestToken change as a fresh present request, but the container RESETS the token to zero after the add field arms (consumption) — that reset unlatched a popover the user had just dismissed and let a stale re-render tick re-present it. Token zero now means consumed, not requested. Also logs checklist popover container-state changes (DEBUG) for lifecycle forensics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Consume pending add-field activation when the checklist popover closes Review finding (Greptile P1 / Cursor): dismissing the first-item popover without committing (Escape, click-away, app deactivation, workspace switch) cleared checklistPopoverWorkspaceId but left the add-field activation token set, leaving the workspace in stale "add requested" state and keeping the empty section mounted invisibly. Any false write through the popover-presented binding now also consumes the activation, and the workspace-switch dismissal path clears the dismissed workspace's token directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Run the popover's window-wide pre-show layout only on hidden-to-shown Review finding (Codex P1): present() re-enters on every parent update tick while the popover is shown, and the root layoutSubtreeIfNeeded() added for same-transaction first presents ran before the isShown guard — synchronously flushing the entire main window's layout on every checklist/title/status change while any todo popover was open. The root layout, identity bump, and initial sizing now all sit behind the isShown guard; shown-popover content and size updates flow through update(model:) -> refreshContent() as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review: keep popover host mounted, hit-test pass-through, coalesced hover Three review findings on the checklist popover: - The empty-section mount condition now includes the popover-presented state: unmounting on token consumption dismantled the popover's anchor mid-presentation, so Return on an empty first-item draft or deleting the last item tore the popover down (and the first add could race the snapshot refresh). - PopoverPointerTracker's view returns nil from hitTest: tracking areas fire from geometry alone, and the full-size background view must never win clicks over the SwiftUI controls it sits behind. - Pointer location now lives in a reference box mutated per mouse event; SwiftUI state (hoveredItemId) is written only when the hovered row actually changes, so per-pixel mouse movement no longer rebuilds every popover row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ContentView.swift (1)
10544-10554: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPrune checklist UI state when a workspace closes.
expandedChecklistWorkspaceIdsandchecklistAddFieldActivationTokensare only cleared on selection changes, so closing/detaching a workspace leaves stale UUID entries behind for the rest of the session. Clear both from the workspace-removal path, or prune them againsttabManager.tabswhen tab IDs change.🤖 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/ContentView.swift` around lines 10544 - 10554, The checklist UI state is not removed when a workspace tab closes, leaving stale UUID entries. Update the workspace-removal path to remove the closed workspace ID from both expandedChecklistWorkspaceIds and checklistAddFieldActivationTokens, while preserving existing selection-change behavior.
🤖 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 `@Sources/Panels/WorkspaceTodoPanel.swift`:
- Around line 34-41: The add-field activation currently relies on a mutable
token plus caller-side coordination; replace this with one pane-owned activation
action/state transition. In Sources/Panels/WorkspaceTodoPanel.swift lines 34-41,
remove addFieldArmToken and armAddField() in favor of the explicit activation
action. In Sources/Workspace+TodoPane.swift lines 75-87, invoke that single
action instead of separately calling focusPanel and armAddField(), preserving
open/focus and add-field activation through one shared path.
In `@Sources/Panels/WorkspaceTodoPanelView.swift`:
- Around line 399-407: Remove the duplicated firstLineCenterOffset helper from
WorkspaceTodoPanelView and reuse the shared implementation established by the
consolidated checklist-row refactor. Also consolidate checkboxSymbolName with
the corresponding implementations in SidebarWorkspaceChecklistPopover and
SidebarWorkspaceChecklistView, preserving identical behavior across all
checklist rows.
In `@Sources/SidebarWorkspaceTodoPopoverHost.swift`:
- Around line 142-147: Replace the split SwiftUI/AppKit visibility ownership
around presentationRequestToken and awaitingDismissAck with one explicit
`@MainActor` coordinator that owns both presentation intent and actual visibility.
Expose read-only state plus presentation and dismissal actions to the SwiftUI
binding and AppKit delegate, and remove the token/latch synchronization and
related mutable side-channel flags while preserving existing presentation and
dismissal triggers.
---
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 10544-10554: The checklist UI state is not removed when a
workspace tab closes, leaving stale UUID entries. Update the workspace-removal
path to remove the closed workspace ID from both expandedChecklistWorkspaceIds
and checklistAddFieldActivationTokens, while preserving existing
selection-change behavior.
🪄 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: 0a16d968-e957-4c2a-8117-6af086a967f0
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (12)
Resources/Localizable.xcstringsSources/ChecklistPopoverPointerTracking.swiftSources/ChecklistSummaryPopoverModifier.swiftSources/ContentView.swiftSources/Panels/WorkspaceTodoPanel.swiftSources/Panels/WorkspaceTodoPanelView.swiftSources/SidebarWorkspaceChecklistPopover.swiftSources/SidebarWorkspaceChecklistView.swiftSources/SidebarWorkspaceTodoPopoverHost.swiftSources/Workspace+TodoPane.swiftSources/WorkspaceTodoFeature.swiftcmux.xcodeproj/project.pbxproj
| /// Bumped when an open-or-focus entry point (checklist popover footer, | ||
| /// palette, CLI) lands on this pane, so the add field re-arms even when | ||
| /// the pane was ALREADY focused and `isFocused` never transitions. | ||
| @Published private(set) var addFieldArmToken: Int = 0 | ||
|
|
||
| func armAddField() { | ||
| addFieldArmToken += 1 | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Route “open/focus and activate add field” through one action.
The panel token and its caller-side increment are two halves of the same lifecycle workaround.
Sources/Panels/WorkspaceTodoPanel.swift#L34-L41: replaceaddFieldArmTokenwith an explicit pane-owned activation action/state transition.Sources/Workspace+TodoPane.swift#L75-L87: invoke that single action instead of separately callingfocusPanelandarmAddField().
As per coding guidelines, use one shared action path and avoid mutable side channels that repair lifecycle ordering.
📍 Affects 2 files
Sources/Panels/WorkspaceTodoPanel.swift#L34-L41(this comment)Sources/Workspace+TodoPane.swift#L75-L87
🤖 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/Panels/WorkspaceTodoPanel.swift` around lines 34 - 41, The add-field
activation currently relies on a mutable token plus caller-side coordination;
replace this with one pane-owned activation action/state transition. In
Sources/Panels/WorkspaceTodoPanel.swift lines 34-41, remove addFieldArmToken and
armAddField() in favor of the explicit activation action. In
Sources/Workspace+TodoPane.swift lines 75-87, invoke that single action instead
of separately calling focusPanel and armAddField(), preserving open/focus and
add-field activation through one shared path.
Source: Coding guidelines
| /// Distance above a text line's baseline to its optical vertical center | ||
| /// (`(ascender + descender) / 2`), so the checkbox's | ||
| /// `.alignmentGuide(.firstTextBaseline)` centers on the item text's FIRST | ||
| /// line specifically — not the whole multi-line block, and not the | ||
| /// baseline itself. | ||
| private var firstLineCenterOffset: CGFloat { | ||
| let font = NSFont.systemFont(ofSize: itemFontSize) | ||
| return (font.ascender + font.descender) / 2 | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Duplicated checklist-row helpers (see consolidated comment).
firstLineCenterOffset and checkboxSymbolName are re-implemented identically here and in SidebarWorkspaceChecklistPopover.swift / SidebarWorkspaceChecklistView.swift. Addressed together in the consolidated comment below.
Also applies to: 492-498
🤖 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/Panels/WorkspaceTodoPanelView.swift` around lines 399 - 407, Remove
the duplicated firstLineCenterOffset helper from WorkspaceTodoPanelView and
reuse the shared implementation established by the consolidated checklist-row
refactor. Also consolidate checkboxSymbolName with the corresponding
implementations in SidebarWorkspaceChecklistPopover and
SidebarWorkspaceChecklistView, preserving identical behavior across all
checklist rows.
| /// Explicit "user asked for this popover" signal (e.g. the checklist | ||
| /// add-field activation token). A change clears the external-dismissal | ||
| /// latch below, so a context-menu/palette request can always re-present | ||
| /// even while the latch is waiting for the container to acknowledge an | ||
| /// AppKit-side close. | ||
| var presentationRequestToken: Int = 0 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Give popover presentation to one lifecycle owner.
The SwiftUI binding, AppKit delegate, presentationRequestToken, and awaitingDismissAck jointly own visibility. The resulting token/latch protocol repairs stale lifecycle ordering while still permitting contradictory states.
Move presentation intent and actual visibility into one explicit @MainActor coordinator; expose immutable state and actions to SwiftUI rather than synchronizing two owners through acknowledgment tokens.
As per coding guidelines, SwiftUI/AppKit bridge lifecycle must remain under one explicit MainActor owner, without mutable flags or side channels that paper over lifecycle and focus races.
Also applies to: 214-270, 303-367
🤖 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/SidebarWorkspaceTodoPopoverHost.swift` around lines 142 - 147,
Replace the split SwiftUI/AppKit visibility ownership around
presentationRequestToken and awaitingDismissAck with one explicit `@MainActor`
coordinator that owns both presentation intent and actual visibility. Expose
read-only state plus presentation and dismissal actions to the SwiftUI binding
and AppKit delegate, and remove the token/latch synchronization and related
mutable side-channel flags while preserving existing presentation and dismissal
triggers.
Source: Coding guidelines
# Conflicts: # .github/swift-file-length-budget.tsv # Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlCommandCoordinator+Debug.swift # Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlDebugContext.swift # Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs+Debug.swift # Resources/Localizable.xcstrings # Sources/ContentView.swift # Sources/KeyboardShortcutSettingsFileStore.swift # Sources/TerminalController+ControlDebugContext.swift # Sources/TerminalController+DebugMethodNames.swift # Sources/Workspace.swift # Sources/WorkspaceSidebarObservation.swift # cmux.xcodeproj/project.pbxproj # cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift
Union of both sides' entries kept stale pre-merge numbers; ratchet each flagged entry to the merged tree's actual length and track the two files the merge pushed over the 500-line threshold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The file-length gate forbids any net growth in >900-line files and any file newly crossing the 500-line threshold. The restore's additions to seven such files are irreducible call-site/persistence code, so offset them by folding this branch's own added blocks into the files' existing dense style and tightening verbose doc comments (content and links preserved). Pure line folds; no behavior, ordering, or declaration changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The branch had extracted the suite to its own file while main updated the in-file copy (hover-lifecycle hang fix removed tests asserting the old force-publish behavior). Keep the branch's file layout with main's current suite content, one definition total; the sharding guard fails on duplicate selector identifiers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@cmuxTests/SidebarWorkspaceRowInteractionStateTests.swift`:
- Line 9: Insert a newline after the documentation comment in
SidebarWorkspaceRowInteractionStateTests so the `@Suite` struct declaration begins
on its own line and remains compiled as a test suite.
🪄 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: 9bf93ce9-6130-49e4-9db6-bfc995cc3cec
📒 Files selected for processing (2)
cmuxTests/SidebarWorkspaceRowInteractionStateTests.swiftcmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift
💤 Files with no reviewable changes (1)
- cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: guard sidebar workspace rows against status circle glyphs Red commit: asserts the sidebar row rendering sources reference no task-status circle glyph (SidebarWorkspaceTaskStatusGlyph/Control, the row-anchored status popover wiring, and the glyph-only snapshot fields). Fails on current main where PR #7790's feature restore resurfaced the circles on pre-existing workspaces; the follow-up commit removes them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Remove status circle glyphs from sidebar workspace rows Sidebar workspace rows no longer render the leading task-status circle (empty = Todo, half-filled = Working/Needs Attention). The circles shipped with workspaces-as-todos (#7216), were removed by the full revert (#7761, 657248a), and came back with the feature restore (#7790, 998e7fb): statusHidden only defaults true for NEW workspaces, so every pre-existing persisted workspace restored to visible/Auto and the circles reappeared on all old rows. PR #7901 is exonerated — its diff never touches the status glyph code. Surgical removal of the row indicator only: - Drop the SidebarWorkspaceTaskStatusGlyphControl block from the row title line in ContentView.swift, its fixed-width slot, and the row-anchored status popover state (statusPopoverWorkspaceId, isStatusPopoverPresented, onStatusPopoverPresentedChange, Equatable). - Drop the glyph-only snapshot fields (taskStatusHasOverride, taskStatusInferred) from SidebarWorkspaceSnapshotBuilder and the context-menu refresh policy; taskStatus stays for the done-row dim. - Delete the now-dead SidebarWorkspaceTaskStatusGlyphControl and the unused WorkspaceTodoActions.toggleDone (glyph option-click). The status feature itself is untouched: context-menu Status submenu, command palette, CLI, shortcuts, checklist, and the todo pane (which keeps its header glyph and status popover) all still work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Normalize project.pbxproj test wiring order Run scripts/normalize-pbxproj.py so the SidebarWorkspaceRowStatusGlyphRemovalTests pbxproj entries sit in sorted position; fixes the workflow-guard-tests normalization check (linux-preflight fails only downstream of it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>


Summary
WorkspaceTodoState.statusHidden's default fromfalsetotrue: newly created workspaces now start with the status glyph hidden (the existing "None" opt-out state) instead ofAuto. The feature is now opt-in per workspace going forward — right-click a workspace → Status to turn it on.Autostate (covered byWorkspaceTodoSnapshotTests.newWorkspaceDefaultsToHiddenStatusWithoutChangingLegacyRestore).Test plan
./scripts/reload.sh --tag rst2nonebuilds green (both pre- and post- merge with latestmain)Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches workspace session persistence, control-socket automation, and sidebar/shortcut routing across many files; behavior is well-tested at the value/coordinator layer but end-to-end UI and agent integrations need careful dogfooding.
Overview
Restores workspaces-as-todos after the prior revert: per-workspace lifecycle status (inferred from agents, PRs, git dirtiness, with optional manual override that auto-clears when inference moves) plus a persisted checklist (cap 50, user/agent origin).
Automation surface: New control-socket domain
workspace.status.*/workspace.todo.*(wired throughControlWorkspaceTodoContexton the app side) and matching CLI:cmux workspace statusand top-levelcmux todo(add,list,check,setwith JSON replace,openpane, etc.), defaulting to the caller’s workspace like other workspace verbs.Product defaults & UX:
WorkspaceTodoState.statusHiddennow defaults totrue, so new workspaces start with no status glyph until the user opts in via Status; legacy sessions without the field still behave as before. Adds a dedicated todo surface kind/pane, sidebar checklist presentation setting (popover vs inline), and shortcuts to mark done, cycle status, and toggle the highlighted checklist item.Supporting changes include coordinator tests, checklist/status value types in
CmuxWorkspaces, session persistence fields for override/checklist, and swift file-length budget bumps for touched large files.Reviewed by Cursor Bugbot for commit c357069. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Restores workspaces‑as‑todos (status lanes + per‑workspace checklist) and makes it opt‑in. New workspaces default to None (status glyph hidden); existing workspaces keep their prior Auto/visible state.
New Features
cmux todoandworkspace.status.*/workspace.todo.*(get/set status, list/mutate checklist, open pane).Bug Fixes
Written for commit f7188c5. Summary will update on new commits.
Summary by CodeRabbit
workspace statusand top-leveltodo(including JSON output and atomictodo set).