Skip to content
Merged
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
89 changes: 89 additions & 0 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3551,6 +3551,9 @@ struct CMUXCLI {
case "reorder-workspaces":
try runReorderWorkspaces(commandArgs: commandArgs, client: client, jsonOutput: jsonOutput, idFormat: idFormat)

case "simulate-sidebar-drag":
try runSimulateSidebarDrag(commandArgs: commandArgs, client: client, jsonOutput: jsonOutput, idFormat: idFormat)

case "workspace-action":
try runWorkspaceAction(commandArgs: commandArgs, client: client, jsonOutput: jsonOutput, idFormat: idFormat, windowOverride: windowId)
case "tab-action":
Expand Down Expand Up @@ -6037,6 +6040,69 @@ struct CMUXCLI {
}
}

private func runSimulateSidebarDrag(
commandArgs: [String],
client: SocketClient,
jsonOutput: Bool,
idFormat: CLIIDFormat
Comment on lines +6052 to +6103

@coderabbitai coderabbitai Bot May 26, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Localize newly introduced CLI-facing strings.

These new error/help/summary strings are hard-coded English. Route them through the project’s localization API and add matching catalog entries for all supported locales.

As per coding guidelines: “For production changes, fail when the diff violates .github/review-bot-rules/full-internationalization.md: user-facing Swift text must use String(localized:defaultValue:) or an equivalent localized API with a matching translated string-catalog entry … for every locale.”

Also applies to: 12112-12131

🤖 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/cmux.swift` around lines 6013 - 6047, The new user-facing strings (the
CLIError messages, the duration/steps validation messages, and the summary
passed to printV2Payload) must be localized instead of hard-coded; replace each
literal string in this block (e.g., the messages thrown where you call CLIError,
the "--duration-ms must be a positive integer"/"--steps must be a positive
integer" texts, and the summary "OK steps=...") with
String(localized:defaultValue:) (or the project’s localization helper) and add
corresponding entries to the app’s string catalog for all supported locales;
ensure you update the same pattern for other occurrences in this file (including
the normalizeWorkspaceHandle/optionValue call sites that produce CLI errors) so
every CLI-facing message uses the localization API and has matching catalog
keys/translations.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Skipping — these are CLI dev-tool diagnostics (CLIError messages and per-command help text), not in-app UI strings. Existing CLI subcommands like reorder-workspace use the same hard-coded English pattern (CLI/cmux.swift:5885,5903). The CLAUDE.md localization rule targets user-facing UI strings; debug-only CLI scaffolding is consistent with the established CLI convention. Happy to revisit if you'd prefer the whole CLI surface go through xcstrings.

— Claude Code

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@azooz2003-bit, that's a clear and well-reasoned distinction — CLI diagnostic/error strings and help text following the existing hard-coded English convention, separate from in-app UI localization through xcstrings. I'll record this to avoid raising the same flag on future CLI subcommands.

Would you like me to resolve this comment?


✏️ Learnings added
Learnt from: azooz2003-bit
Repo: manaflow-ai/cmux PR: 4771
File: CLI/cmux.swift:6013-6047
Timestamp: 2026-05-26T05:16:35.015Z
Learning: In the manaflow-ai/cmux project, CLI-facing strings (CLIError messages, per-subcommand help/usage text, and diagnostic summary output in CLI/cmux.swift) are intentionally kept as hard-coded English and are NOT subject to the xcstrings/`String(localized:)` localization requirement. The localization rule (full-internationalization.md) targets in-app user-facing UI strings only. Existing CLI subcommands such as reorder-workspace follow the same hard-coded English pattern. Do not flag missing localization for CLI error messages, help text, or diagnostic output in CLI/**/*.swift.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.

Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: CLI/cmux.swift:462-499
Timestamp: 2026-03-09T02:08:51.778Z
Learning: For Claude Code session tag extraction in CLI/cmux.swift, in ClaudeHookTagExtractor.extractTags(subtitle:body:), pre-redact sensitive spans (UUIDs, emails, access tokens, filesystem paths, ENV_VAR=..., long numerics) across the combined body+subtitle using unanchored sensitiveSpanPatterns before tokenization. Then tokenize and still filter each token with anchored sensitivePatterns. Rationale: prevents PII/path fragments from slipping into searchable tags after delimiter splitting.

Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.

Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2514
File: CLI/cmux.swift:9698-9706
Timestamp: 2026-04-01T23:08:15.505Z
Learning: When loading/using a user-provided “custom executable” path in the CLI (e.g., from environment/config/UserDefaults), treat it as an executable file path only if it is NOT a directory and is executable. Concretely, check `isDirectory == false` before checking `isExecutableFile`; this avoids accepting directory-valued paths and should allow safe fallback to PATH or bundled/default executables when the candidate is invalid.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3057
File: CLI/cmux.swift:0-0
Timestamp: 2026-04-24T09:07:05.847Z
Learning: In the CLI error-printing codepaths for setup/install (e.g., in runSetupHooks), when displaying an error, prefer printing `String(describing: error)` instead of `error.localizedDescription`. This ensures the full `CLIError.message` via `CustomStringConvertible` is what shows up in CLI output. Apply this same pattern to similar CLI error prints across the CLI Swift sources.

Learnt from: pgbezerra
Repo: manaflow-ai/cmux PR: 3307
File: Sources/cmuxApp.swift:6413-6417
Timestamp: 2026-04-30T11:55:31.575Z
Learning: In this repo (manaflow-ai/cmux), when adding a new Settings section in SwiftUI (e.g., in Sources/cmuxApp.swift or related Views), don’t wire navigation/search with a raw anchor string alone. Instead: (1) create a corresponding SettingsNavigationTarget enum case (e.g., .workspaces); (2) provide the localized title, symbol, search text, and aliases for that case; (3) add/update the matching entry in SettingsSearchIndex so the sidebar/search can navigate to it; and (4) apply .settingsSearchAnchor(SettingsSearchIndex.sectionID(for: <target>)) to the section header. This prevents broken jump-to behavior by ensuring the navigation anchor and the search index stay consistent.

Learnt from: where-is-atin
Repo: manaflow-ai/cmux PR: 3562
File: CLI/cmux.swift:16408-16419
Timestamp: 2026-05-06T00:14:39.636Z
Learning: 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. String(describing: error) preserves the full, structured description and is safer for user-facing and diagnostic messages. Apply this in all error formatting paths; if you need a user-friendly message, consider a separate, explicit mapping to user text rather than relying on localizedDescription.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3626
File: Sources/TabManager.swift:1817-1818
Timestamp: 2026-05-07T08:37:03.967Z
Learning: In this Swift repo (manafow-ai/cmux), when cleaning up stale agent process entries, use `Workspace.clearAgentPID(key:panelId:)` as the single cleanup path. Do not directly mutate `Workspace.statusEntries` or `Workspace.agentPIDs` from outside the dedicated helpers; for example, `TabManager.sweepStaleAgentPIDs` should only call `clearAgentPID` rather than performing its own mutations. This ensures panel-scoped side effects and port/refresh logic run consistently.

Learnt from: psh4607
Repo: manaflow-ai/cmux PR: 3696
File: cmuxTests/ShortcutAndCommandPaletteTests.swift:1716-1771
Timestamp: 2026-05-07T10:56:50.266Z
Learning: In the manaflow-ai/cmux repo, SwiftLint does not enforce a `required_deinit` rule (no project `.swiftlint.yml` in cmux itself, no `required_deinit` in `.github/review-bot-rules/`, and no SwiftLint CI run in `.github/workflows/`). During code reviews, do not raise findings for missing `deinit` on `XCTestCase` subclasses or other Swift classes based on a `required_deinit` rule.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3784
File: README.md:161-161
Timestamp: 2026-05-09T04:48:35.413Z
Learning: In the cmux project, the right-sidebar keyboard shortcut labels were intentionally swapped (per PR `#3784`). Reviewers should NOT flag the ⌘⇧E (Cmd+Shift+E) label as “Open file explorer.” Use these mappings consistently: ⌘⇧E → `focusRightSidebar` with the user-facing label “Toggle right sidebar focus”; ⌘⌥B (Cmd+Option+B) → `toggleFileExplorer` with the user-facing label “Open file explorer.”

) throws {
let windowRaw = optionValue(commandArgs, name: "--window")
let windowHandle = try normalizeWindowHandle(windowRaw, client: client)
guard let windowHandle else {
throw CLIError(message: "simulate-sidebar-drag requires --window <id|ref|index>")
}
guard let fromRaw = optionValue(commandArgs, name: "--from") else {
throw CLIError(message: "simulate-sidebar-drag requires --from <workspace id|ref|index>")
}
guard let toRaw = optionValue(commandArgs, name: "--to") else {
throw CLIError(message: "simulate-sidebar-drag requires --to <workspace id|ref|index>")
}
let fromHandle = try normalizeWorkspaceHandle(fromRaw, client: client, windowHandle: windowHandle)
let toHandle = try normalizeWorkspaceHandle(toRaw, client: client, windowHandle: windowHandle)
guard let fromHandle, let toHandle else {
throw CLIError(message: "simulate-sidebar-drag could not resolve --from / --to to workspace ids")
}

var params: [String: Any] = [
"window_id": windowHandle,
"from_tab_id": fromHandle,
"to_tab_id": toHandle
]
var requestedDurationMs = 1000 // matches server default
var requestedSteps: Int?
if let durationRaw = optionValue(commandArgs, name: "--duration-ms") {
guard let duration = Int(durationRaw), duration > 0 else {
throw CLIError(message: "--duration-ms must be a positive integer")
}
params["duration_ms"] = duration
requestedDurationMs = duration
}
if let stepsRaw = optionValue(commandArgs, name: "--steps") {
guard let steps = Int(stepsRaw), steps > 0 else {
throw CLIError(message: "--steps must be a positive integer")
}
params["steps"] = steps
requestedSteps = steps
}

// The handler blocks until the simulated drag completes
// (duration_ms in the common path; longer when --steps > path
// length because the per-step interval has a 1ms minimum). The
// default 15s socket response timeout would abort long profiling
// runs while the app keeps simulating. Allow generous slack.
let stepBasedMinMs = (requestedSteps ?? 0) // server enforces 1ms min interval
let expectedRuntimeMs = max(requestedDurationMs, stepBasedMinMs)
let responseTimeout = max(30.0, Double(expectedRuntimeMs) / 1000.0 + 10.0)

let payload = try client.sendV2(
method: "debug.sidebar.simulate_drag",
params: params,
responseTimeout: responseTimeout
)
let summary = "OK steps=\(payload["steps"] ?? "?") duration_ms=\(payload["duration_ms"] ?? "?") edge=\(payload["edge"] ?? "?")"
printV2Payload(payload, jsonOutput: jsonOutput, idFormat: idFormat, fallbackText: summary)
}

private func runWorkspaceAction(
commandArgs: [String],
client: SocketClient,
Expand Down Expand Up @@ -12104,6 +12170,28 @@ struct CMUXCLI {
cmux reorder-workspaces --order workspace:1,workspace:11,workspace:31
cmux reorder-workspaces --order workspace:11,workspace:1 --dry-run
""")
case "simulate-sidebar-drag":
return """
Usage: cmux simulate-sidebar-drag --window <id|ref|index> --from <ws> --to <ws> [flags]

Drive deterministic sidebar drag-state mutations against a DEBUG build of
the app, intended for headless profiling under xctrace (see the profile-pr
skill in cmuxterm-hq). Sets dragState.draggedTabId to --from, ticks
dragState.dropIndicator across the rows between --from and --to over
--duration-ms in --steps increments, then clears both. Does NOT commit a
reorder. Only available in DEBUG builds.

Flags:
--window <id|ref|index> Window context (required)
--from <id|ref|index> Workspace to mark as the dragged tab (required)
--to <id|ref|index> Final target neighbor row (required)
--duration-ms <n> Total simulation duration (default: 1000)
--steps <n> Number of indicator updates (default: row count between from and to)

Example:
cmux simulate-sidebar-drag --window window:1 --from workspace:1 --to workspace:25 --duration-ms 2000
cmux simulate-sidebar-drag --window window:1 --from workspace:1 --to workspace:25 --steps 120 --duration-ms 2000
"""
case "workspace-action":
return """
Usage: cmux workspace-action --action <name> [flags]
Expand Down Expand Up @@ -29851,6 +29939,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
sidebar-state [--workspace <id|ref|index>] [--window <id|ref|index>]
set-app-focus <active|inactive|clear>
simulate-app-active
simulate-sidebar-drag --window <id|ref|index> --from <ws> --to <ws> [--duration-ms <n>] [--steps <n>]

# tmux compatibility commands
capture-pane [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--scrollback] [--lines <n>]
Expand Down
61 changes: 59 additions & 2 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9637,10 +9637,46 @@ private final class SidebarTabItemSettingsStore: ObservableObject {
final class SidebarDragState {
var draggedTabId: UUID?
var dropIndicator: SidebarDropIndicator?
/// True while the `debug.sidebar.simulate_drag` debug-only V2 method is
/// driving the drag state. The lifecycle observers honor this by not
/// starting `SidebarDragFailsafeMonitor` (which would otherwise post a
/// `mouse_up_failsafe` clear request immediately since no real mouse is
/// pressed during simulation). DEBUG-only by convention; never set in
/// release flows.
var isSimulated: Bool = false

init() {}
}

#if DEBUG
/// Debug-only registry that exposes the live `SidebarDragState` of each
/// mounted `VerticalTabsSidebar` keyed by `windowId`. The debug-socket
/// `debug.sidebar.simulate_drag` handler reads from this so external
/// profiling tools (e.g. the `profile-pr` skill driving `xctrace`) can
/// generate deterministic drag-state mutations against the running app
/// without HID synthesis.
@MainActor
enum SidebarDragStateRegistry {
private static var statesByWindowId: [UUID: SidebarDragState] = [:]

static func register(windowId: UUID, dragState: SidebarDragState) {
statesByWindowId[windowId] = dragState
}

static func unregister(windowId: UUID) {
statesByWindowId.removeValue(forKey: windowId)
}

static func state(forWindowId windowId: UUID) -> SidebarDragState? {
statesByWindowId[windowId]
}

static func registeredWindowIds() -> [UUID] {
Array(statesByWindowId.keys)
}
}
#endif

/// Per-row drop-indicator visibility, computed by the parent from value
/// inputs only. Takes UUIDs (not `Tab` objects or `SidebarDragState`) so it's
/// trivially unit-testable and the row's view subtree never reads the
Expand Down Expand Up @@ -9967,6 +10003,14 @@ struct VerticalTabsSidebar: View {
modifierKeyMonitor.start()
dragState.draggedTabId = nil
dragState.dropIndicator = nil
// Defensive reset: if a prior simulation died without running
// its teardown (sidebar unmounted mid-loop, app crash, etc.) the
// @State SidebarDragState could carry isSimulated=true into a
// re-mount, which would silently bypass the real-drag failsafe.
dragState.isSimulated = false
#if DEBUG
SidebarDragStateRegistry.register(windowId: windowId, dragState: dragState)
#endif
Comment on lines 10003 to +10013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 isSimulated is not reset in onAppear, unlike draggedTabId and dropIndicator. If the sidebar disappears while a simulation is running, onDisappear calls SidebarDragStateRegistry.unregister, which causes the simulation teardown's v2MainSync guard to early-exit — isSimulated is never cleared. When the sidebar reappears using the same SidebarDragState instance, isSimulated is still true, and the next real HID drag will skip SidebarDragFailsafeMonitor entirely, leaving any stuck drag state unrecoverable.

Suggested change
modifierKeyMonitor.start()
dragState.draggedTabId = nil
dragState.dropIndicator = nil
#if DEBUG
SidebarDragStateRegistry.register(windowId: windowId, dragState: dragState)
#endif
modifierKeyMonitor.start()
dragState.draggedTabId = nil
dragState.dropIndicator = nil
#if DEBUG
dragState.isSimulated = false
SidebarDragStateRegistry.register(windowId: windowId, dragState: dragState)
#endif

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in d441637 — applied your suggested onAppear reset of dragState.isSimulated = false for symmetry with the existing onDisappear cleanup. Defensive against the case where onDisappear didn't run (view destroyed mid-simulation, crash) and the @State SidebarDragState carries the flag into a re-mount, bypassing the real-drag failsafe.

— Claude Code

SidebarDragLifecycleNotification.postStateDidChange(
tabId: nil,
reason: "sidebar_appear"
Expand All @@ -9978,6 +10022,12 @@ struct VerticalTabsSidebar: View {
dragFailsafeMonitor.stop()
dragState.draggedTabId = nil
dragState.dropIndicator = nil
// Clear the simulator flag too so a re-mounted sidebar doesn't
// inherit a stale bypass and skip the real-drag failsafe monitor.
dragState.isSimulated = false
#if DEBUG
SidebarDragStateRegistry.unregister(windowId: windowId)
#endif
SidebarDragLifecycleNotification.postStateDidChange(
tabId: nil,
reason: "sidebar_disappear"
Expand All @@ -9992,8 +10042,15 @@ struct VerticalTabsSidebar: View {
cmuxDebugLog("sidebar.dragState.sidebar tab=\(debugShortSidebarTabId(newDraggedTabId))")
#endif
if newDraggedTabId != nil {
dragFailsafeMonitor.start {
SidebarDragLifecycleNotification.postClearRequest(reason: $0)
// The failsafe monitor probes the real mouse-button state and
// posts `mouse_up_failsafe` if no mouse is held down. That's
// correct for HID-driven drags, but `debug.sidebar.simulate_drag`
// drives the state without any mouse, so skip the monitor when
// a simulated drag is in flight.
if !dragState.isSimulated {
dragFailsafeMonitor.start {
SidebarDragLifecycleNotification.postClearRequest(reason: $0)
}
}
return
}
Expand Down
Loading
Loading