Skip to content

mux: purge per-surface agent/notification tables on close (follow-up to #7584) #7593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lawrencecchen merged 1 commit into from
Jul 8, 2026
Merged

mux: purge per-surface agent/notification tables on close (follow-up to #7584) #7593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions mux/bindings/conformance/fixtures.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@
},
{
"name": "implemented-verbs-json",
"requires": {
"commands": ["run", "wait-for", "send-key", "copy", "ids", "notify", "list-agents", "report-agent"]
},
"steps": [
{
"type": "command",
Expand Down
45 changes: 45 additions & 0 deletions mux/crates/mux-core/src/mux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,16 @@ impl Mux {
record
}

/// Drop the per-surface side tables (`agent_records`,
/// `surface_notifications`) for a surface that has left the tree.
/// `SurfaceId` is monotonic, so without this every closed tab would
/// leak an entry forever and `list-agents` would keep reporting dead
/// surfaces as live agents.
fn purge_surface_side_tables(&self, surface: SurfaceId) {
self.agent_records.lock().unwrap().remove(&surface);
self.surface_notifications.lock().unwrap().remove(&surface);
}
Comment on lines +424 to +427

@greptile-apps greptile-apps Bot Jul 8, 2026

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.

P2 Shutdown Leaves Stale Side Tables

When shutdown() kills the remaining surfaces, it does not call this cleanup path. If the Mux is still queried during teardown, list_agents and surface_notification can still return entries for surfaces that were just killed.


pub fn list_agents(
&self,
surface: Option<SurfaceId>,
Expand Down Expand Up @@ -928,6 +938,7 @@ impl Mux {
(remove_surface(&mut state, target), state.workspaces.is_empty())
};
if let Some(surface) = removed {
self.purge_surface_side_tables(surface.id);
surface.kill();
self.emit(MuxEvent::TreeChanged);
}
Expand All @@ -951,6 +962,7 @@ impl Mux {
};
if !removed.is_empty() {
for surface in removed {
self.purge_surface_side_tables(surface.id);
surface.kill();
}
self.emit(MuxEvent::TreeChanged);
Expand Down Expand Up @@ -1521,6 +1533,39 @@ mod tests {
assert!(mux.list_agents(Some(surface.id), Some(AgentState::Done)).is_empty());
}

#[test]
fn closing_a_surface_purges_agent_and_notification_side_tables() {
let mux = test_mux();
let first = mux.new_workspace(None, None).unwrap();
let pane = mux.with_state(|state| state.pane_of(first.id).unwrap());
// A second tab keeps the workspace alive after `first` closes, so we
// exercise the per-surface purge rather than a full teardown.
let second = mux.new_tab(Some(pane), None, None).unwrap();

mux.report_agent(
first.id,
AgentState::Working,
AgentSource::Socket,
Some("conf".to_string()),
);
mux.post_notification(
"Build".to_string(),
"ok".to_string(),
NotificationLevel::Warning,
Some(first.id),
);
assert_eq!(mux.list_agents(Some(first.id), None).len(), 1);
assert!(mux.surface_notification(first.id).is_some());

mux.close_surface(first.id);

// The dead surface must not linger in either side table.
assert!(mux.list_agents(Some(first.id), None).is_empty());
assert!(mux.list_agents(None, None).is_empty());
assert!(mux.surface_notification(first.id).is_none());
assert!(mux.with_state(|state| state.surfaces.contains_key(&second.id)));
}

#[test]
fn notification_sets_unread_and_clears_when_tab_is_viewed() {
let mux = test_mux();
Expand Down
26 changes: 20 additions & 6 deletions mux/crates/mux-tui/src/ui/sidebar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,25 @@ use ratatui::Frame;
use super::truncate;
use crate::app::{App, Hit};

fn workspace_has_unread(ws: &crate::session::WorkspaceView) -> bool {
/// The color of a workspace's unread indicator, or `None` when nothing is
/// unread. Mirrors the tab-bar severity cue (`error` > `warning` > `info`)
/// so the sidebar dot carries the same meaning as the per-tab marker.
fn workspace_unread_color(
theme: &crate::config::Theme,
ws: &crate::session::WorkspaceView,
) -> Option<Color> {
ws.screens
.iter()
.flat_map(|screen| screen.panes.iter())
.flat_map(|pane| pane.tabs.iter())
.any(|tab| tab.notification.is_some_and(|notification| notification.unread))
.filter_map(|tab| tab.notification.filter(|notification| notification.unread))
.map(|notification| match notification.level {
"error" => (2u8, theme.notification_error),
"warning" => (1, theme.notification_warning),
_ => (0, theme.notification_info),
})
.max_by_key(|(rank, _)| *rank)
.map(|(_, color)| color)
}

pub fn draw(app: &mut App, frame: &mut Frame) {
Expand Down Expand Up @@ -83,10 +96,11 @@ pub fn draw(app: &mut App, frame: &mut Frame) {
buf[(0, y)].set_symbol("▎").set_style(rail_style);
buf[(0, y + 1)].set_symbol("▎").set_style(rail_style);
}
if workspace_has_unread(ws) && content_w > 1 {
let dot_style =
style.fg(app.config.theme.notification_info).add_modifier(Modifier::BOLD);
buf[(0, y)].set_symbol("•").set_style(dot_style);
if content_w > 1 {
if let Some(color) = workspace_unread_color(&app.config.theme, ws) {
let dot_style = style.fg(color).add_modifier(Modifier::BOLD);
buf[(0, y)].set_symbol("•").set_style(dot_style);
}
}
set_line_from(buf, 1, y, &truncate(&ws.name, content_w - 1), style);
hits.push((row_rect(y), Hit::Workspace { index: i, id: ws.id }));
Expand Down
8 changes: 4 additions & 4 deletions mux/spec/cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ The generated CLI requires one of `--index` or `--delta` for `select-tab`, `sele
| `wait-for` | implemented | `--surface <id> --pattern <regex> --timeout-ms <n>` | none | none |
| `run` | implemented | `-- <argv...>` or `--command <cmd>` | `--pane <id>`, `--new-workspace`, `--cwd <path>`, `--name <name>` | surface id |
| `send-key` | implemented | `--surface <id> <key>...` | none | none |
| `copy` | implemented | `--surface <id> --mode screen|selection|scrollback` | none | text |
| `ids` | implemented | none | `--kind workspace|screen|pane|surface` | id lines |
| `notify` | implemented | `--title <title> --body <body>` | `--level info|warning|error`, `--surface <id>` | notification id |
| `copy` | implemented | `--surface <id> --mode screen\|selection\|scrollback` | none | text |
| `ids` | implemented | none | `--kind workspace\|screen\|pane\|surface` | id lines |
| `notify` | implemented | `--title <title> --body <body>` | `--level info\|warning\|error`, `--surface <id>` | notification id |
| `list-agents` | implemented | none | `--surface <id>`, `--state <state>` | agent lines |
| `report-agent` | implemented | `--surface <id> --state <state> --source socket|hook` | `--session <id>` | none |
| `report-agent` | implemented | `--surface <id> --state <state> --source socket\|hook` | `--session <id>` | none |

## Worked Examples

Expand Down
Loading