Skip to content

linux: implement runtime callbacks (close surface, set title, pwd)#237

Merged
Jesssullivan merged 2 commits intomainfrom
sid/close-surface-callback
Apr 18, 2026
Merged

linux: implement runtime callbacks (close surface, set title, pwd)#237
Jesssullivan merged 2 commits intomainfrom
sid/close-surface-callback

Conversation

@Jesssullivan
Copy link
Copy Markdown
Owner

Summary

  • Set surface_config.userdata to the GtkWidget pointer during surface creation
  • Implement onCloseSurface callback: walks tab manager to find the owning panel, removes it, closes empty workspaces
  • Fixes dead shells remaining visible after terminal process exit

Previously onCloseSurface was a no-op stub (identified as a remaining gap from #207). Now libghostty's close notification properly triggers panel cleanup.

Design notes

  • Iterator safety: captures panel ID before calling removePanel to avoid hash map iterator invalidation
  • No double-free: panel.surface is always null (ghostty surface lives on the Surface wrapper, not the panel struct), so removePanel's ghostty_surface_free guard is a no-op — the surface is already freed by libghostty before calling the callback
  • Last-workspace guard: won't close the final workspace, preserving a usable window state

Test plan

  • Code review: no iterator invalidation, no double-free
  • Launch cmux-linux, open terminal, run exit — panel should be removed
  • With multiple workspaces: exit last shell in a workspace → workspace closes
  • Single workspace with single shell: exit → workspace stays (empty but present)

🤖 Generated with Claude Code

When a terminal process exits, libghostty calls close_surface_cb.
Previously this was a no-op, leaving dead shells visible. Now:

- Set surface_config.userdata to the GtkWidget pointer during surface
  creation so the callback can identify the owning panel.
- Walk the tab manager to find the panel matching the widget, remove it,
  and close the workspace if it becomes empty (unless it's the last one).
- Capture panel ID before removal to avoid iterator invalidation on the
  panel hash map.
Wire the onAction callback to dispatch SET_TITLE and PWD actions:

- SET_TITLE: updates the panel title and propagates to workspace tab
  title (unless a custom title is set). Enables terminal escape
  sequences (like shell prompt integration) to update tab names.
- PWD: updates the panel and workspace current_directory, enabling
  shell-integration working directory tracking.

Both use getSurfaceUserdata() to resolve the target surface's widget
pointer and walk the tab manager to find the owning panel.
@Jesssullivan Jesssullivan changed the title linux: implement onCloseSurface for terminal exit cleanup linux: implement runtime callbacks (close surface, set title, pwd) Apr 18, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 18, 2026

Greptile Summary

This PR implements onCloseSurface to clean up terminal panels on process exit, and also adds handleSetTitle/handlePwd action handlers for shell title and directory integration. The close-surface logic is well-structured (iterator-safe, no double-free), but the two new action handlers leak memory on every update.

  • handleSetTitle (line 39): overwrites panel.title without freeing the old allocation — leaks on every shell prompt that sets a title.
  • handlePwd (lines 67–68): same issue for panel.directory and ws.current_directory — leaks on every PWD update from shell integration.

Confidence Score: 4/5

Safe to merge after fixing two memory leaks in the new action handlers; the core close-surface logic is correct.

Two P1 memory leaks are introduced by the new handleSetTitle and handlePwd functions — they accumulate unboundedly during normal terminal use (every shell prompt or directory change). The onCloseSurface implementation itself is correct. Score is 4 pending the leak fixes.

cmux-linux/src/app.zig — handleSetTitle (line 39) and handlePwd (lines 67–68)

Important Files Changed

Filename Overview
cmux-linux/src/app.zig Implements onCloseSurface (panel cleanup on terminal exit), handleSetTitle, and handlePwd. The close-surface logic is correct (iterator-safe, no double-free), but handleSetTitle and handlePwd leak the previous panel.title, panel.directory, and ws.current_directory allocations on every update.
cmux-linux/src/surface.zig Adds surface_config.userdata = gl_area during onRealize so onCloseSurface can match the callback to the owning panel. Change is minimal and correct.

Sequence Diagram

sequenceDiagram
    participant GH as libghostty
    participant CB as app.zig callbacks
    participant WS as Workspace
    participant TM as TabManager

    GH->>CB: onCloseSurface(userdata=GtkWidget*)
    CB->>TM: getTabManager()
    CB->>WS: iterate panels to find widget
    CB->>WS: removePanel(panel_id)
    alt workspace empty && not last
        CB->>TM: closeWorkspace(ws_idx)
        TM->>WS: deinit + destroy
        TM-->>TM: remove AdwTabView page
    end

    GH->>CB: onAction(SET_TITLE)
    CB->>CB: getSurfaceUserdata(target)
    CB->>WS: iterate panels to find widget
    CB->>WS: panel.title = dupe(title_str)
    Note over CB,WS: old panel.title not freed
    CB->>WS: ws.setTitle(title_str)
    CB->>TM: updateTabTitle(ws)

    GH->>CB: onAction(PWD)
    CB->>CB: getSurfaceUserdata(target)
    CB->>WS: iterate panels to find widget
    CB->>WS: panel.directory = dupe(pwd_str)
    Note over CB,WS: old panel.directory not freed
    CB->>WS: ws.current_directory = dupe(pwd_str)
    Note over CB,WS: old ws.current_directory not freed
Loading

Reviews (1): Last reviewed commit: "linux: handle SET_TITLE and PWD actions ..." | Re-trigger Greptile

Comment thread cmux-linux/src/app.zig
if (panel.widget) |pw| {
if (pw == widget) {
// Set panel title
panel.title = ws.alloc.dupe(u8, title_str) catch null;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Memory leak: old panel.title not freed before replacement

panel.title is overwritten on every set_title escape sequence (which fires on each shell prompt change) without freeing the previous allocation. Over a long session this grows unboundedly. Compare with ws.setTitle() which correctly frees first.

Suggested change
panel.title = ws.alloc.dupe(u8, title_str) catch null;
if (panel.title) |old| ws.alloc.free(old);
panel.title = ws.alloc.dupe(u8, title_str) catch null;

Comment thread cmux-linux/src/app.zig
Comment on lines +67 to +68
panel.directory = ws.alloc.dupe(u8, pwd_str) catch null;
ws.current_directory = ws.alloc.dupe(u8, pwd_str) catch null;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Memory leak: old panel.directory / ws.current_directory not freed

Both slices are overwritten without freeing the previous allocation, leaking memory on every shell PWD integration update. Same fix pattern as ws.setTitle():

Suggested change
panel.directory = ws.alloc.dupe(u8, pwd_str) catch null;
ws.current_directory = ws.alloc.dupe(u8, pwd_str) catch null;
if (panel.directory) |old| ws.alloc.free(old);
panel.directory = ws.alloc.dupe(u8, pwd_str) catch null;
if (ws.current_directory) |old| ws.alloc.free(old);
ws.current_directory = ws.alloc.dupe(u8, pwd_str) catch null;

@Jesssullivan Jesssullivan merged commit 3e74c3c into main Apr 18, 2026
24 of 30 checks passed
@Jesssullivan Jesssullivan deleted the sid/close-surface-callback branch April 18, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant