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
2 changes: 1 addition & 1 deletion mux/crates/mux-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub use model::{Node, Pane, Screen, State, Workspace};
pub use mux::{
AgentRecord, AgentSource, AgentState, AppliedLayout, AppliedPane, Direction, LayoutLeafSpec,
LayoutSpec, Mux, MuxEvent, NotificationEvent, NotificationLevel, RunPlacement,
SurfaceNotification, ZoomMode, ZoomState,
SidebarPluginOptions, SidebarPluginStatus, SurfaceNotification, ZoomMode, ZoomState,
};
pub use short_id::assign_short_ids;
pub use surface::{
Expand Down
164 changes: 163 additions & 1 deletion mux/crates/mux-core/src/mux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::collections::HashMap;
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::mpsc::{Receiver, Sender, channel};
use std::sync::{Arc, Mutex};
use std::time::{SystemTime, UNIX_EPOCH};
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};

use crate::browser::{self, BrowserBootstrap, BrowserRuntime};
use crate::layout::{Rect, layout_screen};
Expand Down Expand Up @@ -195,6 +195,28 @@ pub struct ZoomState {
pub zoomed_pane: Option<PaneId>,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SidebarPluginOptions {
pub command: Vec<String>,
pub cwd: Option<String>,
}

#[derive(Debug, Clone)]
pub struct SidebarPluginStatus {
pub surface: Option<SurfaceId>,
pub error: Option<String>,
pub retry_after: Option<Duration>,
}

#[derive(Debug, Default)]
struct SidebarPluginRuntime {
options: Option<SidebarPluginOptions>,
surface: Option<SurfaceId>,
last_error: Option<String>,
failures: u32,
retry_at: Option<Instant>,
}

/// The multiplexer. Shared by frontends and the control socket server.
pub struct Mux {
state: Mutex<State>,
Expand All @@ -206,6 +228,7 @@ pub struct Mux {
browser_runtime: Mutex<Option<Arc<BrowserRuntime>>>,
cell_pixels: Mutex<(u16, u16)>,
default_colors: Mutex<DefaultColors>,
sidebar_plugin: Mutex<SidebarPluginRuntime>,
agent_records: Mutex<HashMap<SurfaceId, AgentRecord>>,
surface_notifications: Mutex<HashMap<SurfaceId, SurfaceNotification>>,
#[cfg(test)]
Expand Down Expand Up @@ -241,6 +264,7 @@ impl Mux {
browser_runtime: Mutex::new(None),
cell_pixels: Mutex::new((8, 16)),
default_colors: Mutex::new(DefaultColors::default()),
sidebar_plugin: Mutex::new(SidebarPluginRuntime::default()),
agent_records: Mutex::new(HashMap::new()),
surface_notifications: Mutex::new(HashMap::new()),
#[cfg(test)]
Expand Down Expand Up @@ -330,6 +354,33 @@ impl Mux {
self.spawn_surface_with_command(cwd, size, None)
}

fn spawn_sidebar_plugin_surface(
self: &Arc<Self>,
options: &SidebarPluginOptions,
size: (u16, u16),
) -> anyhow::Result<Arc<Surface>> {
if options.command.is_empty() {
anyhow::bail!("sidebar plugin command is empty");
}
let id = self.next_id();
let mut opts = self.surface_options.lock().unwrap().clone();
opts.command = Some(options.command.clone());
opts.cwd = options.cwd.clone();
opts.cols = size.0.max(1);
opts.rows = size.1.max(1);
opts.extra_env.push(("CMUX_SIDEBAR".to_string(), "1".to_string()));
#[cfg(test)]
let surface = if self.test_surface_runtime {
Surface::spawn_for_test(id, opts, Arc::downgrade(self))?
} else {
Surface::spawn(id, opts, Arc::downgrade(self))?
};
#[cfg(not(test))]
let surface = Surface::spawn(id, opts, Arc::downgrade(self))?;
self.state.lock().unwrap().surfaces.insert(id, surface.clone());
Ok(surface)
}

Comment on lines +357 to +383

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated surface-spawn boilerplate.

spawn_sidebar_plugin_surface repeats spawn_surface_with's id-generation / #[cfg(test)] spawn dispatch / state-insert block almost verbatim — only the opts field overrides differ. Consider extracting a shared fn finish_spawn(self: &Arc<Self>, id: SurfaceId, opts: SurfaceOptions) -> anyhow::Result<Arc<Surface>> helper that both callers build opts for and delegate to, so future changes to spawn bookkeeping (e.g. additional insert-time side effects) don't need to be kept in sync across two near-identical copies.

♻️ Sketch of a shared helper
+    fn finish_spawn(self: &Arc<Self>, id: u64, opts: SurfaceOptions) -> anyhow::Result<Arc<Surface>> {
+        #[cfg(test)]
+        let surface = if self.test_surface_runtime {
+            Surface::spawn_for_test(id, opts, Arc::downgrade(self))?
+        } else {
+            Surface::spawn(id, opts, Arc::downgrade(self))?
+        };
+        #[cfg(not(test))]
+        let surface = Surface::spawn(id, opts, Arc::downgrade(self))?;
+        self.state.lock().unwrap().surfaces.insert(id, surface.clone());
+        Ok(surface)
+    }
+
     fn spawn_sidebar_plugin_surface(
         self: &Arc<Self>,
         options: &SidebarPluginOptions,
         size: (u16, u16),
     ) -> anyhow::Result<Arc<Surface>> {
         if options.command.is_empty() {
             anyhow::bail!("sidebar plugin command is empty");
         }
         let id = self.next_id();
         let mut opts = self.surface_options.lock().unwrap().clone();
         opts.command = Some(options.command.clone());
         opts.cwd = options.cwd.clone();
         opts.cols = size.0.max(1);
         opts.rows = size.1.max(1);
         opts.extra_env.push(("CMUX_SIDEBAR".to_string(), "1".to_string()));
-        #[cfg(test)]
-        let surface = if self.test_surface_runtime {
-            Surface::spawn_for_test(id, opts, Arc::downgrade(self))?
-        } else {
-            Surface::spawn(id, opts, Arc::downgrade(self))?
-        };
-        #[cfg(not(test))]
-        let surface = Surface::spawn(id, opts, Arc::downgrade(self))?;
-        self.state.lock().unwrap().surfaces.insert(id, surface.clone());
-        Ok(surface)
+        self.finish_spawn(id, opts)
     }
📝 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.

Suggested change
fn spawn_sidebar_plugin_surface(
self: &Arc<Self>,
options: &SidebarPluginOptions,
size: (u16, u16),
) -> anyhow::Result<Arc<Surface>> {
if options.command.is_empty() {
anyhow::bail!("sidebar plugin command is empty");
}
let id = self.next_id();
let mut opts = self.surface_options.lock().unwrap().clone();
opts.command = Some(options.command.clone());
opts.cwd = options.cwd.clone();
opts.cols = size.0.max(1);
opts.rows = size.1.max(1);
opts.extra_env.push(("CMUX_SIDEBAR".to_string(), "1".to_string()));
#[cfg(test)]
let surface = if self.test_surface_runtime {
Surface::spawn_for_test(id, opts, Arc::downgrade(self))?
} else {
Surface::spawn(id, opts, Arc::downgrade(self))?
};
#[cfg(not(test))]
let surface = Surface::spawn(id, opts, Arc::downgrade(self))?;
self.state.lock().unwrap().surfaces.insert(id, surface.clone());
Ok(surface)
}
fn finish_spawn(self: &Arc<Self>, id: u64, opts: SurfaceOptions) -> anyhow::Result<Arc<Surface>> {
#[cfg(test)]
let surface = if self.test_surface_runtime {
Surface::spawn_for_test(id, opts, Arc::downgrade(self))?
} else {
Surface::spawn(id, opts, Arc::downgrade(self))?
};
#[cfg(not(test))]
let surface = Surface::spawn(id, opts, Arc::downgrade(self))?;
self.state.lock().unwrap().surfaces.insert(id, surface.clone());
Ok(surface)
}
fn spawn_sidebar_plugin_surface(
self: &Arc<Self>,
options: &SidebarPluginOptions,
size: (u16, u16),
) -> anyhow::Result<Arc<Surface>> {
if options.command.is_empty() {
anyhow::bail!("sidebar plugin command is empty");
}
let id = self.next_id();
let mut opts = self.surface_options.lock().unwrap().clone();
opts.command = Some(options.command.clone());
opts.cwd = options.cwd.clone();
opts.cols = size.0.max(1);
opts.rows = size.1.max(1);
opts.extra_env.push(("CMUX_SIDEBAR".to_string(), "1".to_string()));
self.finish_spawn(id, opts)
}
🤖 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 `@mux/crates/mux-core/src/mux.rs` around lines 357 - 383,
spawn_sidebar_plugin_surface duplicates the id creation, test/non-test
Surface::spawn dispatch, and state insertion logic already handled by
spawn_surface_with. Extract the shared spawn bookkeeping into a helper such as
finish_spawn on mux::Mux that takes the SurfaceId and SurfaceOptions, performs
the cfg(test) Surface::spawn_for_test vs Surface::spawn selection, inserts into
state.surfaces, and returns the Arc<Surface>; then have
spawn_sidebar_plugin_surface only build its overridden opts and delegate to that
helper.

fn spawn_browser_surface(
self: &Arc<Self>,
url: String,
Expand Down Expand Up @@ -538,6 +589,93 @@ impl Mux {
options.browser_session_name = self.session.clone();
}

pub fn configure_sidebar_plugin(&self, options: Option<SidebarPluginOptions>) {
let old_surface = {
let mut runtime = self.sidebar_plugin.lock().unwrap();
if runtime.options == options {
return;
}
runtime.options = options;
runtime.last_error = None;
runtime.failures = 0;
runtime.retry_at = None;
runtime.surface.take()
};
if let Some(surface) =
old_surface.and_then(|id| self.state.lock().unwrap().surfaces.remove(&id))
{
surface.kill();
self.emit(MuxEvent::SurfaceExited(surface.id));
}
}

pub fn ensure_sidebar_plugin(
self: &Arc<Self>,
cols: u16,
rows: u16,
relaunch: bool,
) -> SidebarPluginStatus {
let now = Instant::now();
let size = (cols.max(1), rows.max(1));
let spawn_options = {
let mut runtime = self.sidebar_plugin.lock().unwrap();
let Some(options) = runtime.options.clone() else {
return SidebarPluginStatus { surface: None, error: None, retry_after: None };
};
if let Some(surface_id) = runtime.surface {
if let Some(surface) = self.surface(surface_id).filter(|surface| !surface.is_dead())
{
drop(runtime);
let _ = self.resize_surface(surface_id, size.0, size.1);
drop(surface);
return SidebarPluginStatus {
surface: Some(surface_id),
error: None,
retry_after: None,
};
}
runtime.surface = None;
}
if let Some(error) = runtime.last_error.clone() {
let retry_after = runtime.retry_at.and_then(|retry_at| {
(retry_at > now).then_some(retry_at.saturating_duration_since(now))
});
if !relaunch || retry_after.is_some() {
return SidebarPluginStatus { surface: None, error: Some(error), retry_after };
}
}
options
};
match self.spawn_sidebar_plugin_surface(&spawn_options, size) {
Ok(surface) => {
let surface_id = surface.id;
{
let mut runtime = self.sidebar_plugin.lock().unwrap();
runtime.surface = Some(surface_id);
runtime.last_error = None;
runtime.failures = 0;
runtime.retry_at = None;
}
self.reap_if_dead(&surface);
SidebarPluginStatus { surface: Some(surface_id), error: None, retry_after: None }
Comment thread
cursor[bot] marked this conversation as resolved.
}
Err(err) => {
let mut runtime = self.sidebar_plugin.lock().unwrap();
runtime.surface = None;
runtime.failures = runtime.failures.saturating_add(1);
let delay = sidebar_retry_delay(runtime.failures);
let message = format!("sidebar plugin failed to start: {err}");
runtime.last_error = Some(message.clone());
runtime.retry_at = Some(now + delay);
SidebarPluginStatus {
surface: None,
error: Some(message),
retry_after: Some(delay),
}
}
}
}
Comment on lines +612 to +677

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm each socket connection is handled on its own thread (so
# concurrent SidebarPlugin commands from different clients are plausible).
rg -n 'thread::Builder|incoming\(\)|accept\(\)' mux/crates/mux-core/src/server.rs -A3

Repository: manaflow-ai/cmux

Length of output: 1575


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant source first, then inspect the sidebar-plugin call path.
ast-grep outline mux/crates/mux-core/src/mux.rs --view expanded | sed -n '1,220p'

printf '\n--- ensure_sidebar_plugin references ---\n'
rg -n 'ensure_sidebar_plugin\(' mux/crates/mux-core/src -A3 -B3

printf '\n--- sidebar plugin command path ---\n'
rg -n 'sidebar-plugin|SidebarPlugin|ensure_sidebar_plugin' mux/crates/mux-core/src/server.rs mux/crates/mux-core/src -A4 -B4

Repository: manaflow-ai/cmux

Length of output: 25539


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the sidebar plugin spawn path and any state registration/cleanup it triggers.
sed -n '307,390p' mux/crates/mux-core/src/mux.rs

printf '\n--- surface insertion/cleanup helpers ---\n'
sed -n '1338,1370p' mux/crates/mux-core/src/mux.rs
sed -n '1813,1848p' mux/crates/mux-core/src/mux.rs

Repository: manaflow-ai/cmux

Length of output: 5710


Serialize sidebar plugin startup mux/crates/mux-core/src/mux.rs:620-660 — ensure_sidebar_plugin releases sidebar_plugin before spawning, so two connection threads can both see no live surface and start separate sidebar processes. The later surface id wins and the other stays in state.surfaces without a cleanup path. Keep the slot claimed under the same lock (or add a “spawning” sentinel) until spawn completes.

🤖 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 `@mux/crates/mux-core/src/mux.rs` around lines 612 - 677, ensure_sidebar_plugin
is racing because it drops sidebar_plugin before spawn_sidebar_plugin_surface,
allowing multiple callers to start duplicate sidebar processes. Keep the
sidebar_plugin slot exclusively claimed under the same lock during startup, or
introduce a spawning sentinel in the runtime state so only one thread can spawn
at a time. Update ensure_sidebar_plugin and the SidebarPluginRuntime flow to
serialize startup, then set surface/last_error/retry_at only after the single
spawn completes.


pub fn set_cell_pixel_size(&self, width_px: u16, height_px: u16) {
let next = (width_px.max(1), height_px.max(1));
{
Expand Down Expand Up @@ -1207,10 +1345,29 @@ impl Mux {
/// reaps the surface out of the tree itself, so frontends only need to
/// drop their render state.
pub fn surface_exited(&self, id: SurfaceId) {
if self.sidebar_surface_exited(id) {
self.emit(MuxEvent::SurfaceExited(id));
return;
}
self.close_surface(id);
self.emit(MuxEvent::SurfaceExited(id));
}

fn sidebar_surface_exited(&self, id: SurfaceId) -> bool {
let mut runtime = self.sidebar_plugin.lock().unwrap();
if runtime.surface != Some(id) {
return false;
}
runtime.surface = None;
runtime.failures = runtime.failures.saturating_add(1);
let delay = sidebar_retry_delay(runtime.failures);
runtime.last_error = Some("sidebar plugin exited".to_string());
runtime.retry_at = Some(Instant::now() + delay);
drop(runtime);
self.state.lock().unwrap().surfaces.remove(&id);
true
}

/// Make `pane` the active pane of its screen (and that screen and
/// workspace active).
pub fn focus_pane(&self, pane: PaneId) -> bool {
Expand Down Expand Up @@ -1585,6 +1742,11 @@ fn now_ms() -> u64 {
.unwrap_or(0)
}

fn sidebar_retry_delay(failures: u32) -> Duration {
let shift = failures.saturating_sub(1).min(5);
Duration::from_secs(1u64 << shift)
}

impl Drop for Mux {
fn drop(&mut self) {
if let Ok(state) = self.state.get_mut() {
Expand Down
22 changes: 20 additions & 2 deletions mux/crates/mux-core/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ use crate::model::{Screen, State};
use crate::platform::{self, transport};
use crate::{
AgentRecord, AgentSource, AgentState, AttachFrame, DefaultColors, Direction, LayoutLeafSpec,
LayoutSpec, Mux, MuxEvent, Node, NotificationLevel, PaneId, Rgb, ScreenId, SplitDir, SurfaceId,
SurfaceKind, SurfaceNotification, WorkspaceId, ZoomMode, assign_short_ids,
LayoutSpec, Mux, MuxEvent, Node, NotificationLevel, PaneId, Rgb, ScreenId, SidebarPluginStatus,
SplitDir, SurfaceId, SurfaceKind, SurfaceNotification, WorkspaceId, ZoomMode, assign_short_ids,
Comment thread
cursor[bot] marked this conversation as resolved.
};

pub const PROTOCOL_VERSION: u32 = 6;
Expand Down Expand Up @@ -85,6 +85,12 @@ enum Command {
ReadScreen {
surface: SurfaceId,
},
SidebarPlugin {
cols: u16,
rows: u16,
#[serde(default)]
relaunch: bool,
},
WaitFor {
surface: SurfaceId,
pattern: String,
Expand Down Expand Up @@ -711,6 +717,15 @@ fn get_surface(mux: &Mux, id: SurfaceId) -> anyhow::Result<Arc<crate::Surface>>
mux.surface(id).ok_or_else(|| anyhow::anyhow!("unknown surface {id}"))
}

fn sidebar_plugin_status_json(status: SidebarPluginStatus) -> Value {
let retry_after_ms = status.retry_after.map(|duration| duration.as_millis() as u64);
json!({
"surface": status.surface,
"error": status.error,
"retry_after_ms": retry_after_ms,
})
}

fn require_pty(surface: &crate::Surface) -> anyhow::Result<()> {
if surface.kind() == SurfaceKind::Pty {
Ok(())
Expand Down Expand Up @@ -921,6 +936,9 @@ fn handle_command(mux: &Arc<Mux>, cmd: Command, writer: &LineWriter) -> anyhow::
let text = surface.try_with_terminal(|t| t.viewport_text())??;
Ok(json!({ "text": text }))
}
Command::SidebarPlugin { cols, rows, relaunch } => {
Ok(sidebar_plugin_status_json(mux.ensure_sidebar_plugin(cols, rows, relaunch)))
}
Command::WaitFor { surface, pattern, timeout_ms } => {
let surface = get_surface(mux, surface)?;
require_pty(&surface)?;
Expand Down
Loading
Loading