Repository navigation
Conversation
…rkspace close - Add SidebarPromptLauncher to sidebar footer: multiline text input that runs `ws [target] <prompt>` as a login shell command, streams output with animated dots, and supports machine selector checkboxes (local/devbox/devbox-1/devbox-2) - Add V2 socket command `sidebar.set_status` (key, value, icon, color, priority) and `sidebar.clear_status` for SSH/remote processes to update sidebar status entries - Add CLI `set-status` subcommand in cmuxd-remote mapping to sidebar.set_status - Add `triggerWsResetIfSlotted()` in TabManager: auto-runs `ws reset wkN` when a slotted workspace is closed, matching against ~/.claude/workon/worktrees/*.json - Remove `nonisolated` from CmuxTop*, WorkspaceRemoteConfiguration for compiler compat - Remove solidSurfaceWidthAdjustment/separatorFadeWidth from tab bar (bonsplit API cleanup) - Update vendor/bonsplit submodule and swift-asn1 version pin Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove nonisolated annotations from module-level constants, enums, and structs that don't require isolation — these were producing warnings under newer Swift/Xcode toolchains. Add daemon/remote/cmuxd-remote to .gitignore so the compiled binary is never accidentally committed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add model selector (claude/cursor) to the sidebar prompt launcher - Add "auto" as the default target option; skip target arg when auto is selected so ws picks the right host automatically - Switch target UI from checkboxes to a compact Picker - Detect workspace creation earlier by watching for "Waiting for" / "✓ Workspace" lines in ws output instead of waiting for full process exit; uses a thread-safe single-fire resume to avoid double-resuming the checked continuation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ensure PromptLauncherModel and NSTextView updates run on the main actor after nonisolated cleanup; mark resolveWSBin nonisolated for pipe handlers. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@maucher is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR introduces a new sidebar prompt launcher that runs external ChangesSidebar Prompt Launcher and Status Command
Swift Concurrency Isolation Audit
Tab Management and Session Persistence Updates
Visual Effects and Configuration Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (7 errors, 1 warning, 1 inconclusive)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
|
Closing — this PR was opened against the wrong repo by mistake. Will open on the fork instead. |
Greptile SummaryThis PR adds a sidebar prompt launcher (with target/model pickers and streaming progress), a
Confidence Score: 3/5Two defects in the main feature path need fixing before merge: the main-actor block in ws binary resolution, and the stale-sidebar-status bug from the inlined key removal. The Sources/ContentView.swift (blocking main-actor call, ObservableObject pattern, unused environment object), Sources/TabManager.swift (key-mapping bypass in stale-PID cleanup, hardcoded fallback string), Resources/Localizable.xcstrings (missing translations for 17 locales) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant SidebarPromptLauncher
participant PromptLauncherModel
participant Process as ws Process
participant cmux as cmux (socket)
User->>SidebarPromptLauncher: submit prompt
SidebarPromptLauncher->>PromptLauncherModel: launch()
PromptLauncherModel->>PromptLauncherModel: startDotAnimation()
PromptLauncherModel->>PromptLauncherModel: resolveWSBin() [blocks main actor if which fallback needed]
PromptLauncherModel->>Process: proc.run()
loop readabilityHandler (background thread)
Process-->>PromptLauncherModel: stdout line
PromptLauncherModel->>PromptLauncherModel: "setLastLogLine (Task @MainActor)"
alt Waiting for... line detected
PromptLauncherModel->>PromptLauncherModel: resume continuation (unlock UI)
end
end
Process-->>PromptLauncherModel: terminationHandler resume()
PromptLauncherModel->>SidebarPromptLauncher: "isLoading = false"
Note over cmux: SSH remote path
participant Remote as cmuxd-remote
Remote->>cmux: sidebar.set_status (key, value, icon, color, priority)
cmux->>cmux: v2SidebarSetStatus ws.statusEntries[key]
Reviews (1): Last reviewed commit: "fix: isolate sidebar prompt launcher on ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| nonisolated static func resolveWSBin() -> String? { | ||
| let home = ProcessInfo.processInfo.environment["HOME"] ?? "" | ||
| for path in ["\(home)/bin/ws", "\(home)/.local/bin/ws", "/usr/local/bin/ws"] | ||
| where FileManager.default.isExecutableFile(atPath: path) { return path } | ||
| let proc = Process() | ||
| proc.executableURL = URL(fileURLWithPath: "/usr/bin/which") | ||
| proc.arguments = ["ws"] | ||
| let pipe = Pipe() | ||
| proc.standardOutput = pipe | ||
| try? proc.run() | ||
| proc.waitUntilExit() | ||
| let out = String(data: pipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8)? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return out?.isEmpty == false ? out : nil | ||
| } |
There was a problem hiding this comment.
proc.waitUntilExit() is a synchronous blocking call. resolveWSBin() is called at the top of runWS(target:model:prompt:) async, which executes on @MainActor because PromptLauncherModel is @MainActor-isolated. If the common-case fast paths (checked paths) all miss, the /usr/bin/which ws subprocess will block the main thread until it exits, freezing the UI for the duration of the lookup.
| nonisolated static func resolveWSBin() -> String? { | |
| let home = ProcessInfo.processInfo.environment["HOME"] ?? "" | |
| for path in ["\(home)/bin/ws", "\(home)/.local/bin/ws", "/usr/local/bin/ws"] | |
| where FileManager.default.isExecutableFile(atPath: path) { return path } | |
| let proc = Process() | |
| proc.executableURL = URL(fileURLWithPath: "/usr/bin/which") | |
| proc.arguments = ["ws"] | |
| let pipe = Pipe() | |
| proc.standardOutput = pipe | |
| try? proc.run() | |
| proc.waitUntilExit() | |
| let out = String(data: pipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8)? | |
| .trimmingCharacters(in: .whitespacesAndNewlines) | |
| return out?.isEmpty == false ? out : nil | |
| } | |
| nonisolated static func resolveWSBin() async -> String? { | |
| let home = ProcessInfo.processInfo.environment["HOME"] ?? "" | |
| for path in ["\(home)/bin/ws", "\(home)/.local/bin/ws", "/usr/local/bin/ws"] | |
| where FileManager.default.isExecutableFile(atPath: path) { return path } | |
| return await withCheckedContinuation { cont in | |
| let proc = Process() | |
| proc.executableURL = URL(fileURLWithPath: "/usr/bin/which") | |
| proc.arguments = ["ws"] | |
| let pipe = Pipe() | |
| proc.standardOutput = pipe | |
| proc.terminationHandler = { _ in | |
| let out = String(data: pipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8)? | |
| .trimmingCharacters(in: .whitespacesAndNewlines) | |
| cont.resume(returning: out?.isEmpty == false ? out : nil) | |
| } | |
| if (try? proc.run()) == nil { cont.resume(returning: nil) } | |
| } | |
| } |
Rule Used: Flag new blocking or timing-based synchronization ... (source)
| for key in keysToRemove { | ||
| tab.clearAgentPID(key: key, clearStatus: true, refreshPorts: false) | ||
| tab.statusEntries.removeValue(forKey: key) | ||
| tab.agentPIDs.removeValue(forKey: key) | ||
| } |
There was a problem hiding this comment.
The replaced clearAgentPID call ran keys through agentStatusKey(forAgentPIDKey:), which strips everything after the first dot when the full key has no direct match in statusEntries. The inlined removal uses the raw key from agentPIDs, so dot-compound keys miss the corresponding status entry entirely — stale badges remain visible in the sidebar. removeAgentPIDOwnership is also skipped, leaving orphaned entries in agentPIDPanelIdsByKey.
| "sidebar.prompt_launcher.placeholder": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", "value": "Prompt…" } }, "ja": { "stringUnit": { "state": "translated", "value": "プロンプト…" } } } }, | ||
| "sidebar.prompt_launcher.send": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", "value": "Send" } }, "ja": { "stringUnit": { "state": "translated", "value": "送信" } } } }, |
There was a problem hiding this comment.
en and ja translations, but the catalog already supports 19 locales — ar, bs, da, de, es, fr, it, km, ko, nb, ok, pl, pt-BR, ru, th, tr, uk, and more. Every other locale will fall back to the English value, violating the full-i18n rule that requires matching entries for every locale already in the touched catalog.
Rule Used: Flag production user-facing text that is not fully... (source)
| if let collapsed, !collapsed.isEmpty { | ||
| return collapsed | ||
| } | ||
| return "Untitled Tab" |
There was a problem hiding this comment.
"Untitled Tab" is a hardcoded English string. It is user-visible text (shown in the "close other tabs" confirmation prompt), so it must be routed through String(localized:defaultValue:) with a matching catalog entry for every supported locale.
| return "Untitled Tab" | |
| return String(localized: "tab.untitled", defaultValue: "Untitled Tab") |
Rule Used: Flag production user-facing text that is not fully... (source)
| @MainActor | ||
| private final class PromptLauncherModel: ObservableObject { |
There was a problem hiding this comment.
PromptLauncherModel is already @MainActor-isolated, so ObservableObject/@Published is the legacy pattern here. The cmux rule flags ObservableObject when @Observable is the correct shape — a single-actor model avoids whole-view invalidation on every @Published change and aligns with the codebase's modernization direction.
| @MainActor | |
| private final class PromptLauncherModel: ObservableObject { | |
| @Observable @MainActor | |
| private final class PromptLauncherModel { |
Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| private struct SidebarPromptLauncher: View { | ||
| @StateObject private var model = PromptLauncherModel() | ||
| @EnvironmentObject var tabManager: TabManager |
There was a problem hiding this comment.
tabManager is declared as an @EnvironmentObject but is never referenced in SidebarPromptLauncher's body or helpers. The unused injection will crash at runtime if the view is shown in any context where TabManager is not in the environment (e.g., a preview or isolated test harness).
| private struct SidebarPromptLauncher: View { | |
| @StateObject private var model = PromptLauncherModel() | |
| @EnvironmentObject var tabManager: TabManager | |
| private struct SidebarPromptLauncher: View { | |
| @StateObject private var model = PromptLauncherModel() |
| func updateNSView(_ scrollView: NSScrollView, context: Context) { | ||
| guard Thread.isMainThread else { | ||
| DispatchQueue.main.async { updateNSView(scrollView, context: context) } | ||
| return | ||
| } | ||
| guard let tv = scrollView.documentView as? NSTextView else { return } |
There was a problem hiding this comment.
updateNSView(_:context:) is always called on the main thread, so the Thread.isMainThread guard and DispatchQueue.main.async re-dispatch are dead code. The async re-dispatch also introduces a one-frame lag and breaks SwiftUI's ordering guarantee — two rapid calls could arrive out of order after a third real update.
| func updateNSView(_ scrollView: NSScrollView, context: Context) { | |
| guard Thread.isMainThread else { | |
| DispatchQueue.main.async { updateNSView(scrollView, context: context) } | |
| return | |
| } | |
| guard let tv = scrollView.documentView as? NSTextView else { return } | |
| func updateNSView(_ scrollView: NSScrollView, context: Context) { | |
| guard let tv = scrollView.documentView as? NSTextView else { return } |
Rule Used: Flag new legacy async patterns in cmux-owned Swift... (source)
1 participant
Summary
wswith optional target/model selection, streams progress into the input field, and unlocks as soon as the new workspace appears in cmux.sidebar.set_status/sidebar.clear_statussocket commands pluscmuxd-remote set-status, and auto-runws resetwhen a slotted workspace is closed.nonisolatedannotations and gitignore the built remote daemon binary.PromptLauncherModelon the main actor for Swift concurrency correctness.Test plan
wsinvocationcursoris prepended to ws argscmuxd-remote set-statusand verify sidebar status entry updatesws reset wkNis triggered./scripts/reload.sh --tag local-customizationsMade with Cursor
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a sidebar Prompt Launcher with target/model pickers that runs
ws, streams progress, and opens the workspace as soon as it appears. Introducessidebar.set_statusover the v2 socket andcmuxd-remote set-statusfor remote status updates, plus autows resetwhen closing slotted workspaces.New Features
auto,local,devbox,devbox-1,devbox-2) and model picker (claude,cursor); runswsvia login shell, streams output with animated dots, and unlocks on early “Waiting for …”/“✓ Workspace …” lines.sidebar.set_status(key, value, icon, color, priority) and CLIcmuxd-remote set-statusto update sidebar status entries from SSH/remote processes.ws reset wkNwhen closing a slotted workspace (matched via~/.claude/workon/worktrees/*.json).Refactors
nonisolatedmarkers; isolatePromptLauncherModelon the main actor; minor thread-safety fixes.daemon/remote/cmuxd-remotebinary; updatePackage.resolvedpin forswift-asn1.Written for commit 8d369a0. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes
Chores