Skip to content

linux(socket): surface.action close_left/close_right/close_others (Sprint A #2) - #227

Merged
Jesssullivan merged 1 commit into
mainfrom
sid/socket-surface-action-close-variants
Apr 18, 2026
Merged

Jesssullivan merged 1 commit into
mainfrom
sid/socket-surface-action-close-variants

Conversation

@Jesssullivan

Copy link
Copy Markdown
Owner

Summary

Promotes the three relative-close `surface.action` variants from stub
"not implemented" errors to working handlers. Stacks on PR #218 which
adds the base `surface.action` dispatcher with rename/pin/mark_read.

Depends on #218 — merge #218 first, then this PR will apply cleanly.

Semantics (matching macOS `v2TabAction`)

  • `close_left`: remove all surfaces left of the anchor in
    `ordered_panels`
  • `close_right`: remove all surfaces right of the anchor
  • `close_others`: remove all surfaces except the anchor
  • Pinned surfaces are skipped in all three (`skipped_pinned` count
    echoed)
  • Focus is forced to the anchor surface after batch removal
  • Split tree and widget tree are rebuilt after mutations

Returns `{action, surface_id, closed, skipped_pinned}` matching the
macOS response shape.

What's added

  • `cmux-linux/src/socket.zig`: ~65 LOC implementing the three close
    actions, replacing the previous "not implemented" stub.
  • `tests_v2/test_surface_action_close_variants.py` covering:
    • `close_right` from middle (removes right siblings)
    • `close_left` from middle (removes left siblings)
    • `close_others` (removes all except anchor)
    • pinned surface is skipped (`skipped_pinned >= 1`)
    • no-op `close_left` at index 0 (`closed: 0`)

Test plan

  • Socket tests CI is green
  • Distro tests CI is green

Refs #220 (Sprint A item #2). Depends on #218.

…others

Promotes the three relative-close surface.action variants from stub
"not implemented" errors to working handlers.

Semantics match macOS v2TabAction:
  - close_left: remove all surfaces left of the anchor in ordered_panels
  - close_right: remove all surfaces right of the anchor
  - close_others: remove all surfaces except the anchor
  - Pinned surfaces are skipped in all three (skipped_pinned count echoed)
  - Focus is forced to the anchor surface after batch removal
  - Split tree and widget tree are rebuilt after mutations

Returns {action, surface_id, closed, skipped_pinned} matching the macOS
response shape.

Adds tests_v2/test_surface_action_close_variants.py covering the full
matrix: close_right from middle, close_left from middle, close_others,
pinned-surface skip, and no-op close_left at index 0.

Refs #220 (Sprint A item #2).
@Jesssullivan
Jesssullivan force-pushed the sid/socket-surface-action-close-variants branch from b5ba0a0 to 5dc7bd1 Compare April 18, 2026 04:48
@Jesssullivan
Jesssullivan merged commit 164a471 into main Apr 18, 2026
2 of 9 checks passed
@Jesssullivan
Jesssullivan deleted the sid/socket-surface-action-close-variants branch April 18, 2026 04:48
@greptile-apps

greptile-apps Bot commented Apr 18, 2026

Copy link
Copy Markdown

Greptile Summary

Implements close_left, close_right, and close_others in surface.action on Linux, replacing the previous "not implemented" stubs (~65 LOC in socket.zig). The logic snapshots target IDs before mutation, skips pinned panels, forces focus to the anchor, and rebuilds the widget tree — matching the macOS v2TabAction contract. Accompanying tests in test_surface_action_close_variants.py cover all three actions plus the pinned-skip and no-op cases.

Confidence Score: 5/5

Safe to merge; both findings are minor P2 style/coverage suggestions that do not block correctness.

The implementation is logically correct — it snapshots IDs before mutation, never removes the anchor, correctly skips pinned panels, and matches the macOS response shape. The two findings (silent OOM on append and a missing symmetric no-op test) are P2 quality improvements, not defects on any reachable path.

No files require special attention.

Important Files Changed

Filename Overview
cmux-linux/src/socket.zig Adds ~65-LOC close_left/close_right/close_others handler; logic is sound but uses catch continue on the append loop which silently undercounts closed on OOM.
tests_v2/test_surface_action_close_variants.py Good coverage of all three actions, pinned-skip, and no-op at index 0; missing a symmetric no-op test for close_right at the last index.

Sequence Diagram

sequenceDiagram
    participant Client
    participant socket.zig as socket.zig (handleSurfaceAction)
    participant Workspace
    participant SplitTree

    Client->>socket.zig: surface.action {action: close_left|close_right|close_others, surface_id, workspace_id}
    socket.zig->>Workspace: find anchor index in ordered_panels
    socket.zig->>Workspace: iterate ordered_panels, snapshot IDs to close (skip anchor, skip pinned)
    loop for each panel in to_close
        socket.zig->>SplitTree: closePane(alloc, root, id)
        SplitTree-->>socket.zig: updated root_node
        socket.zig->>Workspace: removePanel(id)
    end
    socket.zig->>Workspace: focused_panel_id = anchor (target_id)
    socket.zig->>SplitTree: buildWidget(new_root) [unless CMUX_NO_SURFACE]
    socket.zig->>Workspace: sidebar.refresh()
    socket.zig-->>Client: {action, surface_id, closed, skipped_pinned}
Loading

Reviews (1): Last reviewed commit: "linux(socket): implement surface.action ..." | Re-trigger Greptile

Comment thread cmux-linux/src/socket.zig
continue;
}
}
to_close.append(id) catch continue;

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 Silent partial close on append failure

catch continue skips a panel that failed to be appended to to_close and lets the loop proceed, so the final closed count in the response will undercount however many panels were silently dropped. The workspace-level analogue on line 1282 uses catch break to at least stop collecting, which produces a cleaner (if still silent) failure. A more correct fix is to surface the error:

Suggested change
to_close.append(id) catch continue;
to_close.append(id) catch return "{\"error\":\"alloc failed\"}";

Comment on lines +137 to +141
# ── No-op close_left at position 0 ────────────────────────
leftmost = _surface_ids(c, ws)[0]
res5 = _close_action(c, ws, leftmost, "close_left")
if res5.get("closed") != 0:
raise cmuxError(f"close_left at idx 0 should close 0: {res5}")

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 Missing symmetric no-op test for close_right at last index

The no-op case for close_left at position 0 is tested (lines 137-141), but there is no corresponding test for close_right when the anchor is already the rightmost surface (expected closed: 0). Adding a symmetric case would fully mirror the existing no-op coverage and guard against an off-by-one if i > idx is ever changed to i >= idx.

This branch was successfully deployed

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