Move report_pwd fast path off main-thread sync
This commit is contained in:
parent
167431b786
commit
39b110d3b3
3 changed files with 88 additions and 2 deletions
10
CLAUDE.md
10
CLAUDE.md
|
|
@ -95,6 +95,16 @@ tail -f "$(cat /tmp/cmux-last-debug-log-path 2>/dev/null || echo /tmp/cmux-debug
|
|||
- Do not add an app-level display link or manual `ghostty_surface_draw` loop; rely on Ghostty wakeups/renderer to avoid typing lag.
|
||||
- **Submodule safety:** When modifying a submodule (ghostty, vendor/bonsplit, etc.), always push the submodule commit to its remote `main` branch BEFORE committing the updated pointer in the parent repo. Never commit on a detached HEAD or temporary branch — the commit will be orphaned and lost. Verify with: `cd <submodule> && git merge-base --is-ancestor HEAD origin/main`.
|
||||
|
||||
## Socket command threading policy
|
||||
|
||||
- Do not use `DispatchQueue.main.sync` for high-frequency socket telemetry commands (`report_*`, `ports_kick`, status/progress/log metadata updates).
|
||||
- For telemetry hot paths:
|
||||
- Parse and validate arguments off-main.
|
||||
- Dedupe/coalesce off-main first.
|
||||
- Schedule minimal UI/model mutation with `DispatchQueue.main.async` only when needed.
|
||||
- Commands that directly manipulate AppKit/Ghostty UI state (focus/select/open/close/send key/input, list/current queries requiring exact synchronous snapshot) are allowed to run on main actor.
|
||||
- If adding a new socket command, default to off-main handling; require an explicit reason in code comments when main-thread execution is necessary.
|
||||
|
||||
## E2E mac UI tests
|
||||
|
||||
Run UI tests on the UTM macOS VM (never on the host machine). Always run e2e UI tests via `ssh cmux-vm`:
|
||||
|
|
|
|||
|
|
@ -103,6 +103,33 @@ class TerminalController {
|
|||
return currentSorted != nextSorted
|
||||
}
|
||||
|
||||
private struct SocketSurfaceKey: Hashable {
|
||||
let workspaceId: UUID
|
||||
let panelId: UUID
|
||||
}
|
||||
|
||||
private final class SocketFastPathState: @unchecked Sendable {
|
||||
private let queue = DispatchQueue(label: "com.cmux.socket-fast-path")
|
||||
private var lastReportedDirectories: [SocketSurfaceKey: String] = [:]
|
||||
private let maxTrackedDirectories = 4096
|
||||
|
||||
func shouldPublishDirectory(workspaceId: UUID, panelId: UUID, directory: String) -> Bool {
|
||||
let key = SocketSurfaceKey(workspaceId: workspaceId, panelId: panelId)
|
||||
return queue.sync {
|
||||
if lastReportedDirectories[key] == directory {
|
||||
return false
|
||||
}
|
||||
if lastReportedDirectories.count >= maxTrackedDirectories {
|
||||
lastReportedDirectories.removeAll(keepingCapacity: true)
|
||||
}
|
||||
lastReportedDirectories[key] = directory
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static let socketFastPathState = SocketFastPathState()
|
||||
|
||||
nonisolated static func explicitSocketScope(
|
||||
options: [String: String]
|
||||
) -> (workspaceId: UUID, panelId: UUID)? {
|
||||
|
|
@ -117,6 +144,15 @@ class TerminalController {
|
|||
return (workspaceId, panelId)
|
||||
}
|
||||
|
||||
nonisolated static func normalizeReportedDirectory(_ directory: String) -> String {
|
||||
let trimmed = directory.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard !trimmed.isEmpty else { return directory }
|
||||
if trimmed.hasPrefix("file://"), let url = URL(string: trimmed), !url.path.isEmpty {
|
||||
return url.path
|
||||
}
|
||||
return trimmed
|
||||
}
|
||||
|
||||
/// Update which window's TabManager receives socket commands.
|
||||
/// This is used when the user switches between multiple terminal windows.
|
||||
func setActiveTabManager(_ tabManager: TabManager?) {
|
||||
|
|
@ -10752,13 +10788,32 @@ class TerminalController {
|
|||
}
|
||||
|
||||
private func reportPwd(_ args: String) -> String {
|
||||
guard let tabManager else { return "ERROR: TabManager not available" }
|
||||
let parsed = parseOptions(args)
|
||||
guard !parsed.positional.isEmpty else {
|
||||
return "ERROR: Missing path — usage: report_pwd <path> [--tab=X] [--panel=Y]"
|
||||
}
|
||||
|
||||
let directory = parsed.positional.joined(separator: " ")
|
||||
let directory = Self.normalizeReportedDirectory(parsed.positional.joined(separator: " "))
|
||||
|
||||
// Shell integration provides explicit UUID handles for cwd updates.
|
||||
// Keep this hot path off-main and drop no-op reports before scheduling UI work.
|
||||
if let scope = Self.explicitSocketScope(options: parsed.options) {
|
||||
guard Self.socketFastPathState.shouldPublishDirectory(
|
||||
workspaceId: scope.workspaceId,
|
||||
panelId: scope.panelId,
|
||||
directory: directory
|
||||
) else {
|
||||
return "OK"
|
||||
}
|
||||
DispatchQueue.main.async {
|
||||
guard let tabManager = AppDelegate.shared?.tabManagerFor(tabId: scope.workspaceId) else { return }
|
||||
tabManager.updateSurfaceDirectory(tabId: scope.workspaceId, surfaceId: scope.panelId, directory: directory)
|
||||
}
|
||||
return "OK"
|
||||
}
|
||||
|
||||
guard let tabManager else { return "ERROR: TabManager not available" }
|
||||
|
||||
var result = "OK"
|
||||
DispatchQueue.main.sync {
|
||||
guard let tab = resolveTabForReport(args) else {
|
||||
|
|
|
|||
|
|
@ -3114,4 +3114,25 @@ final class TerminalControllerSidebarDedupeTests: XCTestCase {
|
|||
XCTAssertNil(TerminalController.explicitSocketScope(options: ["tab": "workspace:1", "panel": UUID().uuidString]))
|
||||
XCTAssertNil(TerminalController.explicitSocketScope(options: ["tab": UUID().uuidString, "panel": "surface:1"]))
|
||||
}
|
||||
|
||||
func testNormalizeReportedDirectoryTrimsWhitespace() {
|
||||
XCTAssertEqual(
|
||||
TerminalController.normalizeReportedDirectory(" /Users/cmux/project "),
|
||||
"/Users/cmux/project"
|
||||
)
|
||||
}
|
||||
|
||||
func testNormalizeReportedDirectoryResolvesFileURL() {
|
||||
XCTAssertEqual(
|
||||
TerminalController.normalizeReportedDirectory("file:///Users/cmux/project"),
|
||||
"/Users/cmux/project"
|
||||
)
|
||||
}
|
||||
|
||||
func testNormalizeReportedDirectoryLeavesInvalidURLTrimmed() {
|
||||
XCTAssertEqual(
|
||||
TerminalController.normalizeReportedDirectory(" file://bad host "),
|
||||
"file://bad host"
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue