Skip to content

linux(socket): implement workspace.action (rename, pin, color, move, close_*) - #221

Merged
Jesssullivan merged 3 commits into
mainfrom
sid/socket-workspace-action
Apr 18, 2026
Merged

Jesssullivan merged 3 commits into
mainfrom
sid/socket-workspace-action

Conversation

@Jesssullivan

Copy link
Copy Markdown
Owner

Summary

Implements workspace.action on the Linux daemon, mirroring the macOS v2WorkspaceAction dispatcher. Supports 12 sub-actions:

  • Mutation: rename, clear_name, pin, unpin, set_color, clear_color
  • Ordering: move_up, move_down, move_top
  • Bulk close: close_above, close_below, close_others

Recognized macOS actions with no Linux backing field yet (set_description, clear_description, mark_read, mark_unread) return a structured \"action not implemented on linux\" error so callers can detect parity gaps. Unknown actions return \"unsupported action\" with a supported allowlist.

The close_* variants pre-compute the target ID set before mutation so the per-close index shift in tab_manager.closeWorkspace cannot corrupt iteration. Pinned workspaces are skipped — matching macOS semantics.

Adds a pure socket round-trip test (tests_v2/test_workspace_action.py) exercising every supported action, both error paths, and the focused-workspace fallback when no workspace_id is provided. The test pins baseline workspaces before invoking close_* so it cannot disturb workspaces it didn't create, and cleans up by unpinning + closing what it touched.

Refs: cmux issue #220 (Phase 3 Sprint A item 1).

Test plan

Mirrors macOS v2WorkspaceAction (Sources/TerminalController.swift). Wires
the dispatch entry plus a handler that supports the property-mutation and
ordering actions that map onto existing Linux primitives:

  rename, clear_name, pin, unpin, set_color, clear_color,
  move_up, move_down, move_top, close_above, close_below, close_others

Recognized macOS actions that have no Linux backing field yet
(set_description, clear_description, mark_read, mark_unread) return a
structured "action not implemented on linux" error so callers can detect
parity gaps without falling off a cliff. Unknown actions return an
"unsupported action" error with a "supported" allowlist.

The close_above/below/others variants pre-compute the set of target IDs
before mutation to avoid the index-shift bug that would otherwise occur
when closeWorkspace renumbers the workspace list mid-loop. Pinned
workspaces are skipped, matching macOS semantics.

Includes tests_v2/test_workspace_action.py — a pure socket round-trip
test that exercises every supported action, the "not implemented" error
path, the unknown-action error path, and the focused-workspace fallback
when no workspace_id is provided. Cleanup pins baselines so the close_*
suite cannot disturb pre-existing workspaces.

Refs: cmux issue #220 (Phase 3 Sprint A item 1).
@greptile-apps

greptile-apps Bot commented Apr 18, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR implements workspace.action on the Linux daemon, mirroring the macOS v2WorkspaceAction dispatcher with 12 sub-actions (mutation, reorder, and bulk close). The JSON-injection issue flagged in a previous review is correctly addressed with writeJsonString throughout. The pre-computed ID-snapshot approach for close_* correctly handles the index-shift problem caused by sequential closeWorkspace calls.

One concern from a previous review thread remains open: the catch break in the close_* collection loop silently truncates the target set on alloc failure, producing a partial mutation with no indication to the caller.

Confidence Score: 4/5

Safe to merge with one known open concern from a prior review thread.

JSON injection (previously flagged P1) is correctly resolved with writeJsonString. Implementation logic for all 12 actions is sound: insertAssumeCapacity is safe after orderedRemove, ws pointer remains valid because the list stores Workspace pointers, and close_ pre-computation correctly handles index shifts. The catch break silent-truncation issue (flagged in a previous thread) remains unaddressed — a partial close mutation still returns "closed":N with no error signal — which keeps the score at 4 rather than 5.

cmux-linux/src/socket.zig — close_* collection loop catch break

Important Files Changed

Filename Overview
cmux-linux/src/socket.zig Adds handleWorkspaceAction with 12 sub-actions; JSON injection correctly guarded with writeJsonString; catch break on alloc failure (flagged previously) still present in close_* collection loop.
tests_v2/test_workspace_action.py Comprehensive pure socket round-trip test; covers all 12 actions, both error paths, and focused-workspace fallback; baseline workspaces pinned before close_* to prevent test pollution; cleanup handled in finally.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[workspace.action request] --> B{action param?}
    B -- missing --> ERR1[error: missing action]
    B -- present --> C{workspace_id param?}
    C -- present --> D[findWorkspaceById]
    C -- absent --> E[tm.selected_index fallback]
    D -- not found --> ERR2[error: workspace not found]
    D -- found --> F{dispatch on action}
    E -- no selection --> ERR3[error: no workspace]
    E -- found --> F
    F -- rename --> G[dupe title, free old, updateTabTitle\nwriteJsonString response]
    F -- clear_name --> H[free old title, updateTabTitle]
    F -- pin/unpin --> I[set is_pinned, sidebar refresh]
    F -- set_color --> J[dupe color, free old\nwriteJsonString response]
    F -- clear_color --> K[free old color, sidebar refresh]
    F -- move_up --> L[orderedRemove + insertAssumeCapacity idx-1\nre-scan for new index]
    F -- move_down --> M[orderedRemove + insertAssumeCapacity idx+1\nre-scan for new index]
    F -- move_top --> N[orderedRemove + insertAssumeCapacity 0\nindex always 0]
    F -- close_above/below/others --> O[pre-compute target IDs\nskip pinned + operator]
    O --> P[close each by re-finding ID\nafter each index shift]
    P --> Q[writeJsonString response with closed count]
    F -- set_description/clear_description/mark_read/mark_unread --> R[error: not implemented on linux]
    F -- unknown --> S[error: unsupported action\nwith supported list]
Loading

Reviews (3): Last reviewed commit: "linux(socket): name WorkspaceLookup so p..." | Re-trigger Greptile

Comment thread cmux-linux/src/socket.zig Outdated
Comment thread cmux-linux/src/socket.zig
Mirrors the fix on PR #218 (commit 01e3be2) — the workspace.action
handler had the same two patterns that Greptile flagged as P1 on
surface.action:

1. **Use-after-free on alloc failure** — rename and set_color freed
   ws.custom_title / ws.custom_color before attempting the dupe of the
   new value. If dupe fails, the early return leaves the field pointing
   at freed memory. Fix: allocate the new value FIRST, then free the
   old, then assign.

2. **JSON injection in echoed user input** — title, color, and action
   were interpolated directly into the response envelope via `{s}`. A
   value containing `"`, `\\`, or control characters would produce
   malformed JSON the client cannot parse. Fix: build the response with
   ArrayList + writer + writeJsonString for every user-supplied string
   (title, color, action — including the close_*/unimplemented/
   unsupported error envelopes).

Refs: cmux PR #221 (preemptive — same review will surface here).
@Jesssullivan

Copy link
Copy Markdown
Owner Author

Preemptively addressed the same two patterns Greptile flagged on the sibling surface.action PR (#218):

  • Alloc-then-free reorder: rename and set_color now allocate the new value first, then free the old, then assign. No more use-after-free on dupe failure.
  • JSON injection: title, color, and action echoes (including the close_*, unimplemented, and unsupported error envelopes) all use writeJsonString to escape user input. The JSON envelope cannot be broken by a value containing ", \\, or control characters.

Commit: 3b0b92a.

Jesssullivan added a commit that referenced this pull request Apr 18, 2026
The macOS build ships a system.tree RPC that flattens the entire UI hierarchy
into a single response so clients (CLIs, shell prompts, status bars) can avoid
N+1 round-trips. Linux currently exposes the leaf RPCs (window.list,
workspace.list, surface.list, pane.list) but not the composed view.

Add handleSystemTree mirroring the macOS shape:
  active: { workspace_id, surface_id, window_id }   // matches identify
  windows[]: { id, ref, index, workspace_count, selected_workspace_id,
               workspaces[]: { id, ref, index, title, selected, pinned,
                               panes[]: { id, ref, index, focused, surface_count,
                                          selected_surface_id, surfaces[]: {
                                            id, ref, index, index_in_pane, type,
                                            focused, selected, selected_in_pane,
                                            pane_id, pane_ref, title } } } }

Linux uses 1:1 panel:pane today, so each pane has exactly one surface and
pane.id == surface.id. The shape leaves room for future pane grouping
(multiple surfaces per pane) without breaking clients.

The handler is alloc-free in the steady-state error path: every writer.* call
that fails returns the same canonical empty envelope so partial responses
never leak. Workspace and panel titles flow through writeJsonString to keep
user-supplied strings safe.

The new socket round-trip test creates an extra workspace, asserts the
envelope, restores the baseline workspace selection, and closes the
scratch workspace in a finally block so it never leaves stale state.

Sprint A item #5 from #220 (TIN-183 follow-up after #218 / #221 / #222).
CI on this branch failed across every distro with:

    src/socket.zig:936:19: error: incompatible types:
      'socket.findWorkspaceById__struct_7670' and
      'socket.handleWorkspaceAction__struct_7750'

The fallback branch in handleWorkspaceAction constructed an anonymous
struct literal `.{ .ws = ..., .index = ... }`, which Zig treats as a
distinct type from the anonymous struct returned by findWorkspaceById,
breaking peer typing in the `if/else` expression.

Promote the lookup result into a named struct (`WorkspaceLookup`) and
annotate the `found` binding so both arms of the conditional coerce to
the same type. The literal in the else block now constructs the named
type explicitly.

Local regression-only; no behavior change.
@Jesssullivan
Jesssullivan merged commit 57a4ca4 into main Apr 18, 2026
22 of 27 checks passed
@Jesssullivan
Jesssullivan deleted the sid/socket-workspace-action branch April 18, 2026 04:44
Jesssullivan added a commit that referenced this pull request Apr 18, 2026
The macOS build ships a system.tree RPC that flattens the entire UI hierarchy
into a single response so clients (CLIs, shell prompts, status bars) can avoid
N+1 round-trips. Linux currently exposes the leaf RPCs (window.list,
workspace.list, surface.list, pane.list) but not the composed view.

Add handleSystemTree mirroring the macOS shape:
  active: { workspace_id, surface_id, window_id }   // matches identify
  windows[]: { id, ref, index, workspace_count, selected_workspace_id,
               workspaces[]: { id, ref, index, title, selected, pinned,
                               panes[]: { id, ref, index, focused, surface_count,
                                          selected_surface_id, surfaces[]: {
                                            id, ref, index, index_in_pane, type,
                                            focused, selected, selected_in_pane,
                                            pane_id, pane_ref, title } } } }

Linux uses 1:1 panel:pane today, so each pane has exactly one surface and
pane.id == surface.id. The shape leaves room for future pane grouping
(multiple surfaces per pane) without breaking clients.

The handler is alloc-free in the steady-state error path: every writer.* call
that fails returns the same canonical empty envelope so partial responses
never leak. Workspace and panel titles flow through writeJsonString to keep
user-supplied strings safe.

The new socket round-trip test creates an extra workspace, asserts the
envelope, restores the baseline workspace selection, and closes the
scratch workspace in a finally block so it never leaves stale state.

Sprint A item #5 from #220 (TIN-183 follow-up after #218 / #221 / #222).
Jesssullivan added a commit that referenced this pull request Apr 18, 2026
#223)

The macOS build ships a system.tree RPC that flattens the entire UI hierarchy
into a single response so clients (CLIs, shell prompts, status bars) can avoid
N+1 round-trips. Linux currently exposes the leaf RPCs (window.list,
workspace.list, surface.list, pane.list) but not the composed view.

Add handleSystemTree mirroring the macOS shape:
  active: { workspace_id, surface_id, window_id }   // matches identify
  windows[]: { id, ref, index, workspace_count, selected_workspace_id,
               workspaces[]: { id, ref, index, title, selected, pinned,
                               panes[]: { id, ref, index, focused, surface_count,
                                          selected_surface_id, surfaces[]: {
                                            id, ref, index, index_in_pane, type,
                                            focused, selected, selected_in_pane,
                                            pane_id, pane_ref, title } } } }

Linux uses 1:1 panel:pane today, so each pane has exactly one surface and
pane.id == surface.id. The shape leaves room for future pane grouping
(multiple surfaces per pane) without breaking clients.

The handler is alloc-free in the steady-state error path: every writer.* call
that fails returns the same canonical empty envelope so partial responses
never leak. Workspace and panel titles flow through writeJsonString to keep
user-supplied strings safe.

The new socket round-trip test creates an extra workspace, asserts the
envelope, restores the baseline workspace selection, and closes the
scratch workspace in a finally block so it never leaves stale state.

Sprint A item #5 from #220 (TIN-183 follow-up after #218 / #221 / #222).

This branch was successfully deployed

No deployments
gpu-tests — f7e21a75 Deployed Apr 18, 2026 by Jesssullivan via GPU smoke test (honey) #108
distro-tests — f7e21a75 Deployed Apr 18, 2026 by Jesssullivan via Distro package tests (self-hosted KVM) #61
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