Skip to content

linux(socket): Sprint B — close macOS parity gap (74 to 182 methods) - #230

Merged
Jesssullivan merged 6 commits into
mainfrom
sid/socket-sprint-b-core
Apr 18, 2026
Merged

Jesssullivan merged 6 commits into
mainfrom
sid/socket-sprint-b-core

Conversation

@Jesssullivan

Copy link
Copy Markdown
Owner

Summary

  • Implement 4 core methods: surface.send_key, surface.ports_kick, workspace.equalize_splits, debug.terminals
  • Batch-stub 104 remaining macOS methods so callers get structured errors instead of unknown-method failures
  • Dispatch table grows from 74 to 189 entries (macOS has 182 — parity achieved plus extras)
  • workspace.equalize_splits uses proportional tree walk: each split ratio = leafCount(first) / total
  • debug.terminals dumps full terminal metadata (workspace, surface, focus, title, tty_name)
  • All stubs return structured JSON errors distinguishing "not implemented" from "not found"

Test plan

  • Linux CI (Debian no-webkit, Ubuntu, Fedora, Arch, Rocky) compiles
  • tests_v2/test_sprint_b_core_parity.py passes on honey runner
  • workspace.equalize_splits correctly sets ratios with orientation filter
  • debug.terminals returns terminal metadata array
  • Debug/remote/browser stubs return without crashing
  • No regressions in existing socket tests

Closes the 108-method gap from #220 Phase 3 audit.

Core implementations:
- surface.send_key: validated key echo stub (ghostty_surface_key TBD)
- surface.ports_kick: accepts and echoes params (remote not on linux)
- workspace.equalize_splits: proportional ratio equalization via split
  tree walk, supports optional orientation filter
- debug.terminals: full terminal metadata dump (workspace, surface,
  focus, title, tty_name)

Batch stubs (reachable, return structured errors):
- 22 debug introspection methods (debug.layout, debug.sidebar.visible,
  debug.command_palette.*, etc.)
- 6 workspace.remote.* methods
- 76 browser automation methods (browser.click, browser.eval,
  browser.screenshot, etc.) — gated behind has_webkit

Dispatch table: 74 → 189 entries (macOS has 182).
Caught by audit agent: settings.open, feedback.open, feedback.submit,
markdown.open were in macOS dispatch but missing from Sprint B batch.
@greptile-apps

greptile-apps Bot commented Apr 18, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR closes the macOS socket API parity gap on Linux by implementing 4 core handlers (surface.send_key, surface.ports_kick, workspace.equalize_splits, debug.terminals) and batch-stubbing 104 remaining macOS methods, growing the dispatch table from 74 to 189 entries. The four previous review threads flagged the most critical open issues (malformed JSON in handleDebugTerminals, settings.open lying about xdg-open, equalize_splits rejecting single-pane workspaces, and inconsistent workspace_id requirement in ports_kick); those remain unresolved.

  • equalize_splits correctly updates split.ratio in memory, but the GTK real-mode path calls buildWidget (creating orphaned new GtkPaned widgets) instead of calling gtk_paned_set_position on the existing split.paned — unlike handlePaneResize which sets positions directly. The four new handlers are only exercised in CMUX_NO_SURFACE headless mode by CI, so this gap won't be caught until tested on a real GTK display.

Confidence Score: 3/5

Not ready to merge: four P1-class issues from prior review threads remain open, and the equalize_splits GTK-mode update gap is newly identified.

Four issues flagged in previous review threads are still unresolved: (1) handleDebugTerminals mid-entry catch continue produces malformed JSON, (2) settings.open returns opened:true without actually calling xdg-open, (3) equalize_splits errors on single-pane workspaces instead of being a no-op, and (4) ports_kick requires workspace_id while send_key makes it optional. All four are correctness/contract bugs, not style issues. CI runs only in CMUX_NO_SURFACE headless mode, so none of these will be caught before merge.

cmux-linux/src/socket.zig — specifically handleDebugTerminals (JSON corruption), handleSettingsOpen (lying opened:true), handleWorkspaceEqualizeSplits (single-pane error + GTK position update), and handleSurfacePortsKick (mandatory workspace_id inconsistency)

Important Files Changed

Filename Overview
cmux-linux/src/socket.zig Adds 4 real handlers + 104 stubs; 4 open bugs from previous review threads remain (malformed JSON in debug.terminals, settings.open, single-pane equalize_splits, ports_kick workspace_id); new finding: equalize_splits does not call gtk_paned_set_position in real GTK mode
cmux-linux/src/session.zig One-line mechanical fix replacing deprecated std.fmt.formatInt with writer.print for Zig compatibility; safe
tests_v2/test_sprint_b_core_parity.py New test covering send_key, ports_kick, equalize_splits, debug.terminals, and stub reachability; runs in headless (CMUX_NO_SURFACE) mode so GTK-mode behavior is not verified; equalize_splits test only checks equalized=true, not actual ratio values
scripts/run-socket-tests.sh Adds test_sprint_b_core_parity to the Phase 1 candidate gate (non-fatal failures); structure and promotion workflow are correct

Sequence Diagram

sequenceDiagram
    participant Caller as CLI / Test
    participant Socket as SocketServer
    participant Dispatch as dispatch()
    participant Handler as Handler fn
    participant Tree as SplitTree

    Caller->>Socket: JSON-RPC line (e.g. workspace.equalize_splits)
    Socket->>Dispatch: dispatch(alloc, line)
    Dispatch->>Dispatch: parse JSON, extract method + params
    Dispatch->>Handler: handler(alloc, params)

    alt surface.send_key
        Handler->>Handler: resolve workspace (id or selectedWorkspace)
        Handler->>Handler: validate surface_id and panel_type==terminal
        Handler-->>Dispatch: {workspace_id, surface_id, key}
    else surface.ports_kick
        Handler->>Handler: require workspace_id (mandatory)
        Handler->>Handler: resolve surface_id
        Handler-->>Dispatch: {workspace_id, surface_id, kicked:false}
    else workspace.equalize_splits
        Handler->>Handler: resolve workspace
        Handler->>Tree: equalizeSplitNode(root, orientation_filter)
        Tree->>Tree: recurse, set split.ratio = n1/(n1+n2)
        Note over Handler,Tree: real GTK mode calls buildWidget instead of gtk_paned_set_position
        Handler-->>Dispatch: {workspace_id, equalized:true}
    else debug.terminals
        Handler->>Handler: iterate workspaces and panels
        Handler->>Handler: build JSON array (catch continue per field)
        Handler-->>Dispatch: {terminals:[...]}
    else stub methods (104)
        Handler-->>Dispatch: {} or {error:...}
    end

    Dispatch->>Dispatch: formatSuccess(alloc, req_id, result)
    Dispatch-->>Socket: {id:N, ok:true, result:{...}}
    Socket-->>Caller: write response to fd
Loading

Reviews (2): Last reviewed commit: "linux(socket): wire debug.terminal.read_..." | Re-trigger Greptile

Comment thread cmux-linux/src/socket.zig
Comment on lines +3149 to +3151
fn handleSettingsOpen(_: Allocator, _: json.Value) []const u8 {
return "{\"opened\":true,\"path\":\"~/.config/cmux/settings.json\"}";
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 settings.open returns opened:true without attempting to open

The doc comment says "attempts xdg-open on the config path" but the implementation immediately returns {"opened":true,...} without making any syscall. Callers (and tests) that gate behavior on opened:true are silently misled; a file manager or editor won't actually appear. Additionally, the hardcoded path ~/.config/cmux/settings.json ignores $XDG_CONFIG_HOME and the HOME-env-var resolution already used by resolveSocketPath, so it can report the wrong path for non-default configs.

Suggested change
fn handleSettingsOpen(_: Allocator, _: json.Value) []const u8 {
return "{\"opened\":true,\"path\":\"~/.config/cmux/settings.json\"}";
}
fn handleSettingsOpen(_: Allocator, _: json.Value) []const u8 {
// Stub: settings.open is not yet implemented on Linux (no settings UI).
// Returns opened=false so callers know the file was not launched.
return "{\"opened\":false,\"path\":\"~/.config/cmux/settings.json\"}";
}

Comment thread cmux-linux/src/socket.zig
Comment on lines +3020 to +3021
const root = ws.root_node orelse return "{\"error\":\"no split tree\"}";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 equalize_splits errors on single-pane workspace instead of succeeding as a no-op

When ws.root_node is null (a workspace with only one panel, or one that hasn't been split yet), the handler returns {"error":"no split tree"}. Callers that call equalize_splits on any workspace regardless of split state will see a spurious error. Equalizing a workspace with zero or one pane is trivially a no-op that should succeed.

Suggested change
const root = ws.root_node orelse return "{\"error\":\"no split tree\"}";
const root = ws.root_node orelse {
// Nothing to equalize — single-pane workspace is trivially equal.
const ws_hex = formatId(ws.id);
return std.fmt.allocPrint(
alloc,
"{{\"workspace_id\":\"{s}\",\"equalized\":true,\"changed\":0}}",
.{@as([]const u8, &ws_hex)},
) catch "{}";
};

Comment thread cmux-linux/src/socket.zig
Comment thread cmux-linux/src/socket.zig
Comment on lines +2991 to +2993
fn handleSurfacePortsKick(alloc: Allocator, params: json.Value) []const u8 {
const tm = getTabManager() orelse return "{\"error\":\"no tab manager\"}";
const ws_id_str = getParamString(params, "workspace_id") orelse return "{\"error\":\"missing workspace_id\"}";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 surface.ports_kick inconsistently requires workspace_id

handleSurfacePortsKick treats workspace_id as a mandatory parameter (hard error if absent), while the neighboring handleSurfaceSendKey makes it optional and falls back to tm.selectedWorkspace(). Both commands target a surface by workspace+surface ID pair, so the API contract should be consistent. The test in test_sprint_b_core_parity.py even explicitly validates that omitting workspace_id returns an error for ports_kick but not for send_key. If the intent is parity with macOS where both are optional, ports_kick should adopt the same fallback pattern as send_key.

std.fmt.formatInt is not available in all Zig 0.14.x builds (fails on
Arch, Fedora, Rocky, Ubuntu CI). Use w.print("{d}", .{val}) which is
the portable writer API.
Add 11 new candidate tests enabled by Sprint A (PRs #218-#229) and
Sprint B (PR #230) socket handlers. These run non-fatally under
CMUX_TEST_PHASE1=1 to observe green runs before promoting to baseline.

New candidates: surface.action rename/close/new/reload, workspace.action,
auth.login, system.tree, notification.create_for_target, app.simulate_active,
surface.report_tty, pane.resize, sprint_b_core_parity.
…used from stubs

debug.layout: serializes split tree to JSON {type, orientation, ratio,
panel_id} hierarchy — enables test introspection of pane geometry.

debug.sidebar.visible: returns {visible:true} (Linux sidebar is always
visible, not toggleable like macOS).

debug.terminal.is_focused: checks focused panel type, returns
{focused:bool}.
…dler

Same operation, different dispatch path. Avoids test failures from
debug-prefixed callers hitting the empty stub.
@Jesssullivan
Jesssullivan merged commit 525c59c into main Apr 18, 2026
22 of 27 checks passed

This branch was successfully deployed

No deployments
gpu-tests — 8857501c Deployed Apr 18, 2026 by Jesssullivan via SSH proxy e2e (honey) #65
distro-tests — 8857501c Deployed Apr 18, 2026 by Jesssullivan via Distro package tests (self-hosted KVM) #87
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