Skip to content

Fix TUI shells remaining open after exit #9003

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

Closed
iarbpairs wants to merge 9 commits into from
Closed

Fix TUI shells remaining open after exit #9003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9deb085
fix(tui): close established shells on exit
iarbpairs Jul 27, 2026
78361d5
fix(tui): complete exited surface cleanup
iarbpairs Jul 27, 2026
513b0c1
Merge remote-tracking branch 'upstream/main' into fix/tui-shell-exit
iarbpairs Jul 27, 2026
7f37ecb
fix(tui): refresh topology before exit cleanup
iarbpairs Jul 28, 2026
45ed2c7
Merge remote-tracking branch 'upstream/main' into fix/tui-shell-exit
iarbpairs Jul 28, 2026
01f8275
test(tui): cover stale pointer and resize races
iarbpairs Jul 28, 2026
88ec011
fix(tui): ignore stale lifecycle input and resize work
iarbpairs Jul 28, 2026
29b0030
test(tui): cover ordered exit close decision
iarbpairs Jul 28, 2026
83d9c10
fix(tui): decide exited surface close on worker
iarbpairs Jul 28, 2026
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
11 changes: 11 additions & 0 deletions cmux-tui/bindings/go/events_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ func TestParseTitleChangedIncludesAuthoritativeTitle(t *testing.T) {
}
}

func TestParseSurfaceExitedExposesHostedRuntime(t *testing.T) {
event, ok := parseEvent(map[string]any{
"event": "surface-exited",
"surface": float64(7),
"runtime_ms": float64(321),
}).(SurfaceEvent)
if !ok || event.RuntimeMS == nil || *event.RuntimeMS != 321 {
t.Fatalf("surface exit = %#v", event)
}
}

func TestParseResizedAcceptsProtocolV6DataField(t *testing.T) {
event, ok := parseEvent(map[string]any{
"event": "resized",
Expand Down
5 changes: 3 additions & 2 deletions cmux-tui/bindings/go/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,9 @@ type OverflowEvent struct {
func (OverflowEvent) EventName() string { return "overflow" }

type SurfaceEvent struct {
Event string `json:"event"`
Surface uint64 `json:"surface"`
Event string `json:"event"`
Surface uint64 `json:"surface"`
RuntimeMS *uint64 `json:"runtime_ms"`
}

func (e SurfaceEvent) EventName() string { return e.Event }
Expand Down
8 changes: 7 additions & 1 deletion cmux-tui/bindings/java/src/com/cmux/CmuxEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ static CmuxEvent from(Map<String, Object> raw) {
raw.get("surface") instanceof Number ? CmuxClient.asLong(raw.get("surface")) : null
);
case "surface-output", "surface-exited", "bell", "detached" ->
new SurfaceEvent(event, CmuxClient.asLong(raw.get("surface")));
new SurfaceEvent(
event,
CmuxClient.asLong(raw.get("surface")),
raw.get("runtime_ms") instanceof Number
? CmuxClient.asLong(raw.get("runtime_ms"))
: null
);
case "title-changed" -> new TitleChangedEvent(
CmuxClient.asLong(raw.get("surface")),
raw.get("title") instanceof String title ? title : null
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/bindings/java/src/com/cmux/SurfaceEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package com.cmux;

public record SurfaceEvent(String event, long surface) implements CmuxEvent {}
public record SurfaceEvent(String event, long surface, Long runtimeMs) implements CmuxEvent {}
6 changes: 6 additions & 0 deletions cmux-tui/bindings/java/tests/com/cmux/JsonTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ public static void main(String[] args) {
(Map<String, Object>) Json.parse("{\"event\":\"title-changed\",\"surface\":7}")
);
assertEquals(null, legacyTitle.title(), "legacy title event title");
SurfaceEvent surfaceExit = (SurfaceEvent) CmuxEvent.from(
(Map<String, Object>) Json.parse(
"{\"event\":\"surface-exited\",\"surface\":7,\"runtime_ms\":321}"
)
);
assertEquals(321L, surfaceExit.runtimeMs(), "hosted surface exit runtime");
CmuxEvent layoutEvent = CmuxEvent.from(
(Map<String, Object>) Json.parse("{\"event\":\"layout-changed\",\"screen\":7}")
);
Expand Down
2 changes: 2 additions & 0 deletions cmux-tui/bindings/python/cmux/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,7 @@ class Event:
error: Optional[str] = None
retry_after_ms: Optional[int] = None
reservation_id: Optional[int] = None
runtime_ms: Optional[int] = None

@property
def bytes_data(self) -> Optional[bytes]:
Expand Down Expand Up @@ -1265,4 +1266,5 @@ def _parse_event(value: Dict[str, Any]) -> Event:
error=value.get("error"),
retry_after_ms=value.get("retry_after_ms"),
reservation_id=value.get("reservation_id"),
runtime_ms=value.get("runtime_ms"),
)
13 changes: 13 additions & 0 deletions cmux-tui/bindings/python/tests/test_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ def test_legacy_title_changed_keeps_title_optional(self) -> None:
self.assertEqual(event.surface, 7)
self.assertIsNone(event.title)

def test_surface_exit_exposes_hosted_runtime(self) -> None:
event = _parse_event(
{
"event": "surface-exited",
"surface": 7,
"runtime_ms": 321,
}
)

self.assertEqual(event.event, "surface-exited")
self.assertEqual(event.surface, 7)
self.assertEqual(event.runtime_ms, 321)

def test_overflow_exposes_recovery_fields(self) -> None:
event = _parse_event(
{
Expand Down
16 changes: 16 additions & 0 deletions cmux-tui/bindings/rust/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,8 @@ fn default_true() -> bool {
#[derive(Debug, Clone, Deserialize)]
pub struct SurfaceEvent {
pub surface: u64,
#[serde(default)]
pub runtime_ms: Option<u64>,
Comment on lines +614 to +615

@greptile-apps greptile-apps Bot Jul 27, 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 Binding event schemas remain inconsistent

The server and Rust binding add runtime_ms, but the Go, Python, TypeScript, and Java typed surface-exited models and conformance fixtures remain unchanged. Those consumers silently lose the metadata and cannot implement the documented established-shell versus startup-failure policy without bypassing their typed APIs.

Knowledge Base Used: cmux-tui

@iarbpairs iarbpairs Jul 27, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 78361d5. runtime_ms now exists in the canonical spec and Rust, Go, Python, TypeScript, and Java event models, with parser/type coverage in every binding.

}

#[derive(Debug, Clone, Deserialize)]
Expand Down Expand Up @@ -1627,6 +1629,20 @@ mod tests {
));
}

#[test]
fn surface_exited_decodes_hosted_runtime() {
let event = parse_event(serde_json::json!({
"event": "surface-exited",
"surface": 7,
"runtime_ms": 321,
}));

assert!(matches!(
event,
Event::SurfaceExited(SurfaceEvent { surface: 7, runtime_ms: Some(321) })
));
}

#[test]
fn resized_decodes_protocol_v6_data_field() {
let event = parse_event(serde_json::json!({
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/bindings/typescript/src/protocol/events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export interface SurfaceResizeFailedEvent {
retry_after_ms: number | null;
reservation_id?: number | null;
}
export interface SurfaceExitedEvent { event: "surface-exited"; surface: Id }
export interface SurfaceExitedEvent { event: "surface-exited"; surface: Id; runtime_ms?: number | null }
export interface TitleChangedEvent { event: "title-changed"; surface: Id; title?: string }
export interface BellEvent { event: "bell"; surface: Id }

Expand Down
8 changes: 8 additions & 0 deletions cmux-tui/bindings/typescript/test/protocol-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
RenderAttachEvent,
RenderDeltaEvent,
RenderStateEvent,
SurfaceExitedEvent,
Screen,
Tree,
TreeDeltaEvent,
Expand Down Expand Up @@ -156,6 +157,13 @@ const stackLayout = {
} as const satisfies import("../src/browser.js").Layout;
void stackLayout;

const hostedSurfaceExit = {
event: "surface-exited",
surface: 7,
runtime_ms: 321,
} satisfies SurfaceExitedEvent;
void hostedSurfaceExit;

function surfaceFromKnownEvent(event: KnownCmuxEvent): number | undefined {
switch (event.event) {
case "surface-output":
Expand Down
29 changes: 14 additions & 15 deletions cmux-tui/crates/cmux-tui-core/src/event_bus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,9 @@ impl MuxEventFilter {
impl SurfaceSessionScope {
fn accepts(&mut self, event: &MuxEvent) -> bool {
match event {
MuxEvent::SurfaceOutput(surface)
| MuxEvent::SurfaceExited(surface)
| MuxEvent::Bell(surface) => *surface == self.surface,
MuxEvent::SurfaceResized { surface, .. }
MuxEvent::SurfaceOutput(surface) | MuxEvent::Bell(surface) => *surface == self.surface,
MuxEvent::SurfaceExited { surface, .. }
| MuxEvent::SurfaceResized { surface, .. }
| MuxEvent::SurfaceResizeFailed { surface, .. }
| MuxEvent::TitleChanged { surface, .. }
| MuxEvent::ScrollChanged { surface, .. } => *surface == self.surface,
Expand Down Expand Up @@ -237,7 +236,7 @@ impl MuxEventMailbox {
state.surface_output_sequences.insert(surface, sequence);
state.surface_outputs.insert(sequence, surface);
}
MuxEvent::SurfaceExited(surface) => {
MuxEvent::SurfaceExited { surface, runtime_ms } => {
if let Some(previous) = state.title_sequences.remove(&surface) {
state.titles.remove(&previous);
}
Expand All @@ -248,14 +247,14 @@ impl MuxEventMailbox {
self.changed.notify_all();
return false;
}
state.events.push_back((sequence, MuxEvent::SurfaceExited(surface)));
state.events.push_back((sequence, MuxEvent::SurfaceExited { surface, runtime_ms }));
}
MuxEvent::Empty => {
let mut terminal_events = state
.events
.iter()
.filter(|(_, event)| {
matches!(event, MuxEvent::SurfaceExited(_))
matches!(event, MuxEvent::SurfaceExited { .. })
|| matches!(
event,
MuxEvent::TreeDelta(delta)
Expand Down Expand Up @@ -448,14 +447,14 @@ mod tests {
broadcaster.emit(MuxEvent::TitleChanged { surface: 1, title: "old".into() });
broadcaster.emit(MuxEvent::Bell(2));
broadcaster.emit(MuxEvent::TitleChanged { surface: 1, title: "latest".into() });
broadcaster.emit(MuxEvent::SurfaceExited(3));
broadcaster.emit(MuxEvent::SurfaceExited { surface: 3, runtime_ms: None });

assert!(matches!(events.recv().unwrap(), MuxEvent::Bell(2)));
assert!(matches!(
events.recv().unwrap(),
MuxEvent::TitleChanged { surface: 1, title } if title.as_ref() == "latest"
));
assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceExited(3)));
assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceExited { surface: 3, .. }));
}

#[test]
Expand All @@ -464,9 +463,9 @@ mod tests {
let events = broadcaster.subscribe();

broadcaster.emit(MuxEvent::TitleChanged { surface: 4, title: "gone".into() });
broadcaster.emit(MuxEvent::SurfaceExited(4));
broadcaster.emit(MuxEvent::SurfaceExited { surface: 4, runtime_ms: None });

assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceExited(4)));
assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceExited { surface: 4, .. }));
assert!(matches!(events.try_recv(), Err(TryRecvError::Empty)));
}

Expand All @@ -481,12 +480,12 @@ mod tests {
broadcaster.emit(MuxEvent::SurfaceOutput(1));
broadcaster.emit(MuxEvent::SurfaceOutput(3));
}
broadcaster.emit(MuxEvent::SurfaceExited(4));
broadcaster.emit(MuxEvent::SurfaceExited { surface: 4, runtime_ms: None });

assert!(matches!(events.recv().unwrap(), MuxEvent::Bell(2)));
assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceOutput(1)));
assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceOutput(3)));
assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceExited(4)));
assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceExited { surface: 4, .. }));
assert!(matches!(events.try_recv(), Err(TryRecvError::Empty)));
}

Expand All @@ -496,9 +495,9 @@ mod tests {
let events = broadcaster.subscribe();

broadcaster.emit(MuxEvent::SurfaceOutput(4));
broadcaster.emit(MuxEvent::SurfaceExited(4));
broadcaster.emit(MuxEvent::SurfaceExited { surface: 4, runtime_ms: None });

assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceExited(4)));
assert!(matches!(events.recv().unwrap(), MuxEvent::SurfaceExited { surface: 4, .. }));
assert!(matches!(events.try_recv(), Err(TryRecvError::Empty)));
}

Expand Down
26 changes: 20 additions & 6 deletions cmux-tui/crates/cmux-tui-core/src/mux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,13 @@ pub enum MuxEvent {
},
/// A surface's child exited. Hosted terminals remain in the tree as an
/// addressable Exited tab until an explicit close tombstones them; local
/// terminals have already been reaped when this arrives.
SurfaceExited(SurfaceId),
/// terminals have already been reaped when this arrives. `runtime_ms` is
/// present for hosted terminals and lets frontends match Ghostty's normal-
/// exit versus startup-failure policy.
SurfaceExited {
surface: SurfaceId,
runtime_ms: Option<u64>,
},
TitleChanged {
surface: SurfaceId,
title: Arc<str>,
Expand Down Expand Up @@ -3882,7 +3887,7 @@ impl Mux {
old_surface.and_then(|id| self.state.lock().unwrap().surfaces.remove(&id))
{
surface.kill();
self.emit(MuxEvent::SurfaceExited(surface.id));
self.emit(MuxEvent::SurfaceExited { surface: surface.id, runtime_ms: None });
}
}

Expand Down Expand Up @@ -6396,8 +6401,12 @@ impl Mux {
/// terminals remain stable, renderable Exited tabs until an explicit
/// close; local surfaces are removed immediately.
pub fn surface_exited(&self, id: SurfaceId) {
self.surface_exited_with_runtime(id, None);
}

pub(crate) fn surface_exited_with_runtime(&self, id: SurfaceId, runtime_ms: Option<u64>) {
if self.sidebar_surface_exited(id) {
self.emit(MuxEvent::SurfaceExited(id));
self.emit(MuxEvent::SurfaceExited { surface: id, runtime_ms: None });
return;
}
if let Some(surface) = self.surface(id)
Expand All @@ -6410,11 +6419,16 @@ impl Mux {
return;
}
self.emit(MuxEvent::TreeChanged);
self.emit(MuxEvent::SurfaceExited(id));
self.emit(MuxEvent::SurfaceExited {
surface: id,
// Hosts predating runtime metadata still represent a process
// that reached the live stream, not a local/browser reap.
runtime_ms: Some(runtime_ms.unwrap_or(u64::MAX)),
});
return;
}
self.remove_surface_after_registry(id);
self.emit(MuxEvent::SurfaceExited(id));
self.emit(MuxEvent::SurfaceExited { surface: id, runtime_ms: None });
Comment on lines +6422 to +6431

@coderabbitai coderabbitai Bot Jul 28, 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.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider naming the u64::MAX sentinel.

Hosts predating runtime metadata still represent a process that reached the live stream, not a local/browser reap. The logic is correct, but the bare u64::MAX magic value could be extracted into a named constant (e.g. UNKNOWN_HOSTED_RUNTIME_MS) to make the intent self-documenting at the call site and any future comparison sites.

🤖 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 `@cmux-tui/crates/cmux-tui-core/src/mux.rs` around lines 6422 - 6431, Define a
named constant for the unknown hosted runtime sentinel and replace the bare
u64::MAX in the SurfaceExited emission path with that constant. Place the
constant near the relevant mux runtime metadata definitions so future
comparisons can reuse the same symbol.

}

fn mark_hosted_surface_exited(
Expand Down
6 changes: 5 additions & 1 deletion cmux-tui/crates/cmux-tui-core/src/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5770,7 +5770,11 @@ fn subscribed_event_json(event: &MuxEvent) -> Value {
"retry_after_ms": retry_after_ms,
"reservation_id": reservation_id,
}),
MuxEvent::SurfaceExited(id) => json!({"event": "surface-exited", "surface": id}),
MuxEvent::SurfaceExited { surface, runtime_ms } => json!({
"event": "surface-exited",
"surface": surface,
"runtime_ms": runtime_ms,
}),
MuxEvent::TitleChanged { surface, title } => {
json!({"event": "title-changed", "surface": surface, "title": title.as_ref()})
}
Expand Down
Loading