marketing-shibata50/cmux
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix SSH review regressions

Browse source
This commit is contained in:
Lawrence Chen 2026-03-12 05:24:12 -07:00
parent d7353d3aa1
commit a75faa82f1
5 changed files with 328 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

102
Resources/Localizable.xcstrings
View file

@ -61540,6 +61540,108 @@
}
}
},
"sidebar.remote.help.connected": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "SSH connected to %@"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "SSH は %@ に接続済み"
}
}
}
},
"sidebar.remote.help.connecting": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "SSH connecting to %@"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "SSH は %@ に接続中"
}
}
}
},
"sidebar.remote.help.error": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "SSH error for %@"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "%@ の SSH エラー"
}
}
}
},
"sidebar.remote.help.errorWithDetail": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "SSH error for %@: %@"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "%@ の SSH エラー: %@"
}
}
}
},
"sidebar.remote.help.disconnected": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "SSH disconnected from %@"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "SSH は %@ から切断済み"
}
}
}
},
"sidebar.remote.help.targetFallback": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "remote host"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "リモートホスト"
}
}
}
},
"sidebar.workspace.moveDownAction": {
"extractionState": "manual",
"localizations": {

51
Sources/ContentView.swift
View file

@ -10477,20 +10477,59 @@ private struct TabItemView: View, Equatable {
}
private var remoteStateHelpText: String {
let target = tab.remoteDisplayTarget ?? "remote host"
let target = tab.remoteDisplayTarget ?? String(
localized: "sidebar.remote.help.targetFallback",
defaultValue: "remote host"
)
let detail = tab.remoteConnectionDetail?.trimmingCharacters(in: .whitespacesAndNewlines)
switch tab.remoteConnectionState {
case .connected:
return "SSH connected to \(target)"
return String(
format: String(
localized: "sidebar.remote.help.connected",
defaultValue: "SSH connected to %@"
),
locale: .current,
target
)
case .connecting:
return "SSH connecting to \(target)"
return String(
format: String(
localized: "sidebar.remote.help.connecting",
defaultValue: "SSH connecting to %@"
),
locale: .current,
target
)
case .error:
if let detail, !detail.isEmpty {
return "SSH error for \(target): \(detail)"
return String(
format: String(
localized: "sidebar.remote.help.errorWithDetail",
defaultValue: "SSH error for %@: %@"
),
locale: .current,
target,
detail
)
}
return "SSH error for \(target)"
return String(
format: String(
localized: "sidebar.remote.help.error",
defaultValue: "SSH error for %@"
),
locale: .current,
target
)
case .disconnected:
return "SSH disconnected from \(target)"
return String(
format: String(
localized: "sidebar.remote.help.disconnected",
defaultValue: "SSH disconnected from %@"
),
locale: .current,
target
)
}
}
private func moveWorkspaces(_ workspaceIds: [UUID], toWindow windowId: UUID) {

61
Sources/TabManager.swift
View file

@ -19,22 +19,31 @@ enum NewWorkspacePlacement: String, CaseIterable, Identifiable {
var displayName: String {
switch self {
case .top:
return "Top"
return String(localized: "workspace.placement.top", defaultValue: "Top")
case .afterCurrent:
return "After current"
return String(localized: "workspace.placement.afterCurrent", defaultValue: "After current")
case .end:
return "End"
return String(localized: "workspace.placement.end", defaultValue: "End")
}
}
var description: String {
switch self {
case .top:
return "Insert new workspaces at the top of the list."
return String(
localized: "workspace.placement.top.description",
defaultValue: "Insert new workspaces at the top of the list."
)
case .afterCurrent:
return "Insert new workspaces directly after the active workspace."
return String(
localized: "workspace.placement.afterCurrent.description",
defaultValue: "Insert new workspaces directly after the active workspace."
)
case .end:
return "Append new workspaces to the bottom of the list."
return String(
localized: "workspace.placement.end.description",
defaultValue: "Append new workspaces to the bottom of the list."
)
}
}
}
@ -732,7 +741,7 @@ class TabManager: ObservableObject {
}
var isFindVisible: Bool {
selectedTerminalPanel?.searchState != nil
selectedTerminalPanel?.searchState != nil || focusedBrowserPanel?.searchState != nil
}
var canUseSelectionForFind: Bool {
@ -740,13 +749,17 @@ class TabManager: ObservableObject {
}
func startSearch() {
guard let panel = selectedTerminalPanel else { return }
if panel.searchState == nil {
panel.searchState = TerminalSurface.SearchState()
if let panel = selectedTerminalPanel {
if panel.searchState == nil {
panel.searchState = TerminalSurface.SearchState()
}
NSLog("Find: startSearch workspace=%@ panel=%@", panel.workspaceId.uuidString, panel.id.uuidString)
NotificationCenter.default.post(name: .ghosttySearchFocus, object: panel.surface)
_ = panel.performBindingAction("start_search")
return
}
NSLog("Find: startSearch workspace=%@ panel=%@", panel.workspaceId.uuidString, panel.id.uuidString)
NotificationCenter.default.post(name: .ghosttySearchFocus, object: panel.surface)
_ = panel.performBindingAction("start_search")
focusedBrowserPanel?.startFind()
}
func searchSelection() {
@ -760,11 +773,21 @@ class TabManager: ObservableObject {
}
func findNext() {
_ = selectedTerminalPanel?.performBindingAction("search:next")
if let panel = selectedTerminalPanel {
_ = panel.performBindingAction("search:next")
return
}
focusedBrowserPanel?.findNext()
}
func findPrevious() {
_ = selectedTerminalPanel?.performBindingAction("search:previous")
if let panel = selectedTerminalPanel {
_ = panel.performBindingAction("search:previous")
return
}
focusedBrowserPanel?.findPrevious()
}
@discardableResult
@ -774,7 +797,12 @@ class TabManager: ObservableObject {
}
func hideFind() {
selectedTerminalPanel?.searchState = nil
if let panel = selectedTerminalPanel {
panel.searchState = nil
return
}
focusedBrowserPanel?.hideFind()
}
@discardableResult
@ -1234,6 +1262,7 @@ class TabManager: ObservableObject {
sentryBreadcrumb("workspace.close", data: ["tabCount": tabs.count - 1])
AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: workspace.id)
workspace.teardownAllPanels()
workspace.teardownRemoteConnection()
unwireClosedBrowserTracking(for: workspace)

45
daemon/remote/cmd/cmuxd-remote/cli.go
View file

@ -226,7 +226,7 @@ func execV2(socketPath string, spec *commandSpec, args []string, jsonOutput bool
if jsonOutput {
fmt.Println(resp)
} else {
fmt.Println("OK")
fmt.Println(defaultRelayOutput(resp))
}
return 0
}
@ -308,11 +308,52 @@ func runBrowserRelay(socketPath string, args []string, jsonOutput bool, refreshA
if jsonOutput {
fmt.Println(resp)
} else {
fmt.Println("OK")
fmt.Println(defaultRelayOutput(resp))
}
return 0
}
func defaultRelayOutput(resp string) string {
var result any
if err := json.Unmarshal([]byte(resp), &result); err != nil {
trimmed := strings.TrimSpace(resp)
if trimmed == "" {
return "OK"
}
return trimmed
}
if relayResultIsEmpty(result) {
return "OK"
}
switch typed := result.(type) {
case string:
return typed
default:
encoded, err := json.MarshalIndent(typed, "", " ")
if err != nil {
return "OK"
}
return string(encoded)
}
}
func relayResultIsEmpty(result any) bool {
switch typed := result.(type) {
case nil:
return true
case map[string]any:
return len(typed) == 0
case []any:
return len(typed) == 0
case string:
return typed == ""
default:
return false
}
}
// flagToParamKey maps a CLI flag name to its JSON-RPC param key.
func flagToParamKey(key string) string {
switch key {

93
daemon/remote/cmd/cmuxd-remote/cli_test.go
View file

@ -16,6 +16,33 @@ import (
"time"
)
func captureStdout(t *testing.T, fn func()) string {
t.Helper()
original := os.Stdout
reader, writer, err := os.Pipe()
if err != nil {
t.Fatalf("pipe stdout: %v", err)
}
os.Stdout = writer
defer func() {
os.Stdout = original
}()
fn()
if err := writer.Close(); err != nil {
t.Fatalf("close stdout writer: %v", err)
}
output, err := io.ReadAll(reader)
if err != nil {
t.Fatalf("read stdout: %v", err)
}
if err := reader.Close(); err != nil {
t.Fatalf("close stdout reader: %v", err)
}
return string(output)
}
// startMockSocket creates a Unix socket that accepts one connection,
// reads a line, and responds with the given canned response.
func startMockSocket(t *testing.T, response string) string {
@ -88,6 +115,46 @@ func startMockV2Socket(t *testing.T) string {
return sockPath
}
func startMockV2TCPSocketWithResult(t *testing.T, result any) string {
t.Helper()
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("failed to listen on TCP: %v", err)
}
t.Cleanup(func() { ln.Close() })
go func() {
for {
conn, err := ln.Accept()
if err != nil {
return
}
go func(conn net.Conn) {
defer conn.Close()
buf := make([]byte, 4096)
n, _ := conn.Read(buf)
if n == 0 {
return
}
var req map[string]any
if err := json.Unmarshal(buf[:n], &req); err != nil {
_, _ = conn.Write([]byte(`{"ok":false,"error":{"code":"parse","message":"bad json"}}` + "\n"))
return
}
resp := map[string]any{
"id": req["id"],
"ok": true,
"result": result,
}
payload, _ := json.Marshal(resp)
_, _ = conn.Write(append(payload, '\n'))
}(conn)
}
}()
return ln.Addr().String()
}
// startMockTCPSocket creates a TCP listener that responds with a canned response.
func startMockTCPSocket(t *testing.T, response string) string {
t.Helper()
@ -391,6 +458,32 @@ func TestCLIListWorkspacesV2(t *testing.T) {
}
}
func TestCLIListWorkspacesV2DefaultOutputShowsResult(t *testing.T) {
sockPath := startMockV2TCPSocketWithResult(t, map[string]any{"method": "workspace.list", "params": map[string]any{}})
output := captureStdout(t, func() {
code := runCLI([]string{"--socket", sockPath, "list-workspaces"})
if code != 0 {
t.Fatalf("list-workspaces should return 0, got %d", code)
}
})
if !strings.Contains(output, "\"method\": \"workspace.list\"") {
t.Fatalf("expected default output to include result payload, got %q", output)
}
}
func TestCLINotifyDefaultOutputPrintsOKForEmptyResult(t *testing.T) {
sockPath := startMockV2TCPSocketWithResult(t, map[string]any{})
output := captureStdout(t, func() {
code := runCLI([]string{"--socket", sockPath, "notify", "--body", "hi"})
if code != 0 {
t.Fatalf("notify should return 0, got %d", code)
}
})
if strings.TrimSpace(output) != "OK" {
t.Fatalf("expected empty-result command to print OK, got %q", output)
}
}
func TestCLIRPCPassthrough(t *testing.T) {
sockPath := startMockV2Socket(t)
code := runCLI([]string{"--socket", sockPath, "rpc", "system.capabilities"})