Conversation
surface.report_tty and surface.ports_kick blocked the calling socket thread with DispatchQueue.main.sync, violating the documented socket command threading policy for high-frequency telemetry commands. Both handlers now follow the same off-main-parse + main.async-mutate shape as workspace.set_status/workspace.report_meta_block: surface resolution and the model mutation run entirely inside a fire-and-forget DispatchQueue.main.async block, and the JSON-RPC response acknowledges immediately by echoing the request instead of a value resolved on main. This changes the response contract: the ok result no longer echoes the surface resolved on main (e.g. the focused surface when surface_id is omitted) and an unknown explicit surface_id no longer returns a synchronous not_found error, since both would require blocking on main. Updated TerminalControllerSocketSecurityTests accordingly. Refs #82.
…ller Two private copies of responderChainContains existed in TerminalController.swift, one under #if DEBUG and one under #if !DEBUG, and neither was ever called from this file. The live implementations used by BrowserPanel.swift and BrowserPanelView.swift are unaffected. Refs #103.
TerminalController.swift covered ~10 v2 socket command domains in a single 9,530-line file. Following the precedent of TerminalController+BrowserAutomation.swift, split each command domain's handler bodies (verbatim, no dispatch/behavior changes) into its own extension file: - TerminalController+Window.swift (window.*) - TerminalController+Workspace.swift (workspace.* CRUD/remote/action/tab.action) - TerminalController+Telemetry.swift (off-main-parse + main.async-mutate report_*/ports_kick/set_status/log/progress/sidebar-metadata commands) - TerminalController+Surface.swift (surface.*) - TerminalController+Pane.swift (pane.*) - TerminalController+Notification.swift (notification.*) - TerminalController+System.swift (system.*/settings.*/feedback.*/app.*) - TerminalController+Debug.swift (debug.* plus their private implementation helpers) The dispatch switch, the capabilities array, socket transport/lifecycle/auth, and helpers genuinely shared across 3+ domains (v2Ok/v2Error/v2Ref family, v2UUID/v2String param parsing, v2ResolveTabManager/v2ResolveWorkspace, orderedPanels/parseSplitDirection/readTerminalTextForSnapshot, which are also called from AppDelegate+UITestCmdClick.swift, GhosttyTerminalView+Mouse.swift, and Workspace.swift) stay in the main file, which is now 2,752 lines. Every handler function reachable from the dispatch switch was widened from private to internal (drop-private only, no signature/behavior changes) since the switch — which stays in the main file per the split's constraints — calls them via self. The same widening was applied to a handful of shared low-level helpers/properties now called cross-file: tabManager, socketPath, socketFastPathState, v2TabRef, v2StringArray, v2StringMap, v2ActionKey, v2UUIDAny, v2LocatePane, v2Double, v2IntArray, v2HasNonNullParam, v2StrictInt, v2PanelType, socketCommandAllowsInAppFocusMutations() (instance overload — was shadowed by an internal static overload of the same name once the private one became invisible cross-file), tailTerminalLines, and parseSidebarMetadataFormat. No other code changes. Registered the 8 new files in GhosttyTabs.xcodeproj/project.pbxproj (PBXFileReference + PBXBuildFile + group + Sources build phase entries, IDs prefixed NRTC). Refs #96.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Fixes the telemetry threading violation and splits the 9,615-line TerminalController into domain files. Closes two nuclear-review issues.
Summary
Review flag
The #82 fix changes the response contract for those two commands: the ok result now echoes the request instead of the main-resolved surface, and an unknown explicit surface_id no longer returns a synchronous not_found (returning it would require blocking on main). The only external caller found (CLI+SSH relay bootstrap) discards the response. Two socket-security tests were updated to observe the now-async mutation.
Fixes #82. Fixes #96.
Test Plan