Skip to content

fix: avoid main thread starvation from synchronous socket telemetry commands #1562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
EtanHey wants to merge 1 commit into from
Closed

fix: avoid main thread starvation from synchronous socket telemetry commands #1562

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 26 additions & 41 deletions Sources/TerminalController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13687,17 +13687,27 @@ class TerminalController {
reportArgs: String,
options: [String: String]
) -> (tabId: UUID?, error: String?) {
var tabId: UUID?
// Off-main fast-path: honor explicit --tab UUID without touching the main thread.
// High-frequency agent/shell flows typically provide explicit IDs.
if let rawTab = options["tab"]?.trimmingCharacters(in: .whitespacesAndNewlines),
!rawTab.isEmpty {
guard let explicitId = UUID(uuidString: rawTab) else {
return (nil, "ERROR: Invalid tab id '\(rawTab)'")
}
return (explicitId, nil)
}

// Fallback: resolve the currently-selected tab on main as a short, bounded hop.
var selectedId: UUID?
DispatchQueue.main.sync {
if let tab = resolveTabForReport(reportArgs) {
tabId = tab.id
selectedId = tab.id
}
}
if let tabId {
return (tabId, nil)
if let selectedId {
return (selectedId, nil)
}
let error = options["tab"] != nil ? "ERROR: Tab not found" : "ERROR: No tab selected"
return (nil, error)
return (nil, "ERROR: No tab selected")
Comment on lines +13692 to +13710

@coderabbitai coderabbitai Bot Mar 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve --tab index routing here.

This fast-path now rejects every non-UUID --tab, but the existing resolver still accepts numeric tab indices. Calls like report_meta ... --tab=0 / set_status ... --tab=1 used to work and will now fail here, and index misses now fall through to "ERROR: No tab selected" instead of "ERROR: Tab not found".

💡 Suggested fix 
     private func resolveTabIdForSidebarMutation(
         reportArgs: String,
         options: [String: String]
     ) -> (tabId: UUID?, error: String?) {
-        // Off-main fast-path: honor explicit --tab UUID without touching the main thread.
-        // High-frequency agent/shell flows typically provide explicit IDs.
-        if let rawTab = options["tab"]?.trimmingCharacters(in: .whitespacesAndNewlines),
-           !rawTab.isEmpty {
-            guard let explicitId = UUID(uuidString: rawTab) else {
-                return (nil, "ERROR: Invalid tab id '\(rawTab)'")
-            }
-            return (explicitId, nil)
+        let rawTab = options["tab"]?.trimmingCharacters(in: .whitespacesAndNewlines)
+        let hasExplicitTab = rawTab?.isEmpty == false
+
+        // Off-main fast-path: honor explicit UUIDs without touching the main thread.
+        if let rawTab, !rawTab.isEmpty {
+            if let explicitId = UUID(uuidString: rawTab) {
+                return (explicitId, nil)
+            }
+            // Keep numeric-index routing on the existing resolveTabForReport path.
+            if Int(rawTab) == nil {
+                return (nil, "ERROR: Invalid tab id '\(rawTab)'")
+            }
         }
 
         // Fallback: resolve the currently-selected tab on main as a short, bounded hop.
         var selectedId: UUID?
         DispatchQueue.main.sync {
             if let tab = resolveTabForReport(reportArgs) {
                 selectedId = tab.id
             }
         }
         if let selectedId {
             return (selectedId, nil)
         }
-        return (nil, "ERROR: No tab selected")
+        return (nil, hasExplicitTab ? "ERROR: Tab not found" : "ERROR: No tab selected")
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/TerminalController.swift` around lines 13692 - 13710, The fast-path
currently rejects any non-UUID --tab and skips index-based routing; change the
logic around options["tab"] so that if rawTab is not a UUID you attempt to parse
it as an integer index and resolve the corresponding tab (preserving the old
numeric-index behavior) before falling back to the main-thread
resolveTabForReport(reportArgs). Specifically: when options["tab"] exists and
trimming yields rawTab, try UUID(uuidString:), else try Int(rawTab) and lookup
the tab by index (returning a "Tab not found" error if the index is out of
range), and only if neither UUID nor numeric index yields a tab continue to the
existing DispatchQueue.main.sync fallback that calls
resolveTabForReport(reportArgs).

}

private func tabForSidebarMutation(id: UUID) -> Tab? {
Expand Down Expand Up @@ -14356,46 +14366,21 @@ class TerminalController {
}
ports.append(port)
}
let normalizedPorts = Array(Set(ports)).sorted()

var result = "OK"
DispatchQueue.main.sync {
guard let tab = resolveTabForReport(args) else {
result = parsed.options["tab"] != nil ? "ERROR: Tab not found" : "ERROR: No tab selected"
return
}

let validSurfaceIds = Set(tab.panels.keys)
tab.pruneSurfaceMetadata(validSurfaceIds: validSurfaceIds)

let panelArg = parsed.options["panel"] ?? parsed.options["surface"]
let surfaceId: UUID
if let panelArg {
if panelArg.isEmpty {
result = "ERROR: Missing panel id — usage: report_ports <port1> [port2...] [--tab=X] [--panel=Y]"
return
}
guard let parsedId = UUID(uuidString: panelArg) else {
result = "ERROR: Invalid panel id '\(panelArg)'"
return
}
surfaceId = parsedId
} else {
guard let focused = tab.focusedPanelId else {
result = "ERROR: Missing panel id (no focused surface)"
return
}
surfaceId = focused
}

guard validSurfaceIds.contains(surfaceId) else {
result = "ERROR: Panel not found '\(surfaceId.uuidString)'"
// Use the shared helper to parse/resolve off-main and schedule mutation async,
// avoiding main thread starvation from synchronous socket telemetry (see #1550).
return schedulePanelMetadataMutation(
args: args,
options: parsed.options,
missingPanelUsage: "report_ports <port1> [port2...] [--tab=X] [--panel=Y]"
) { tab, surfaceId in
guard Self.shouldReplacePorts(current: tab.surfaceListeningPorts[surfaceId], next: normalizedPorts) else {
return
}

tab.surfaceListeningPorts[surfaceId] = ports
tab.surfaceListeningPorts[surfaceId] = normalizedPorts
tab.recomputeListeningPorts()
}
return result
}

private func reportPwd(_ args: String) -> String {
Expand Down