From b92251bdf5b8d5f4c8ea7a578d737152db47b0ac Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 10:58:34 -0300 Subject: [PATCH 01/19] fix: build palette command indexes without trapping on duplicate ids --- Sources/ContentView.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 3a682aa6d..84fa93db8 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -3069,7 +3069,10 @@ struct ContentView: View { includeSurfaces: includeSurfaces, commandsContext: commandsContext ) - commandPaletteController.commandPaletteSearchCommandsByID = Dictionary(uniqueKeysWithValues: entries.map { ($0.id, $0) }) + // `uniquingKeysWith` (not `uniqueKeysWithValues:`) guards against a crash if two entries + // ever share an id -- e.g. Unicode-normalization-equal saved-layout filenames on a + // non-APFS mount -- rather than trapping the whole palette. + commandPaletteController.commandPaletteSearchCommandsByID = Dictionary(entries.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) let searchCorpus = entries.map { entry in CommandPaletteSearchCorpusEntry( payload: entry.id, @@ -3079,7 +3082,7 @@ struct ContentView: View { ) } commandPaletteController.commandPaletteSearchCorpus = searchCorpus - commandPaletteController.commandPaletteSearchCorpusByID = Dictionary(uniqueKeysWithValues: searchCorpus.map { ($0.payload, $0) }) + commandPaletteController.commandPaletteSearchCorpusByID = Dictionary(searchCorpus.map { ($0.payload, $0) }, uniquingKeysWith: { first, _ in first }) commandPaletteController.cachedCommandPaletteScope = scope commandPaletteController.cachedCommandPaletteFingerprint = fingerprint } From 456ca2399e08a8c14988135c6073bc7ebc6b1c26 Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 10:58:50 -0300 Subject: [PATCH 02/19] test: cover session reliability regressions --- .../TabManagerSessionSnapshotTests.swift | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/programaTests/TabManagerSessionSnapshotTests.swift b/programaTests/TabManagerSessionSnapshotTests.swift index ace002624..3615994a5 100644 --- a/programaTests/TabManagerSessionSnapshotTests.swift +++ b/programaTests/TabManagerSessionSnapshotTests.swift @@ -105,6 +105,139 @@ final class TabManagerSessionSnapshotTests: XCTestCase { XCTAssertEqual(restored.worktreeChildren(of: restoredParent).map(\.id), [restoredChild.id]) } + /// Regression for M9 duplicate-repository parent targeting: two workspaces open the same + /// repo path, and the worktree child's recorded owner is the *second* one (not the first + /// in `tabs`, and not whichever one a directory-based re-derivation would prefer). Pre-#336 + /// restore rebuilt worktree parents only from folder membership (`worktreeFolderId`); an + /// ordinary, non-folder parent like `second` here was never restored at all, so this would + /// have failed by leaving `restoredChild.worktreeParentWorkspaceId` nil. The current restore + /// (`TabManager+SessionPersistence.swift`) must instead honor the persisted + /// `worktreeParentWorkspaceId` by workspace identity, landing the child under `second`. + func testWorktreeChildRestoresUnderRecordedParentAmongDuplicateRepoPathWorkspaces() throws { + let manager = TabManager() + let first = try XCTUnwrap(manager.selectedWorkspace) + first.currentDirectory = "/tmp/project" + first.setCustomTitle("First") + let second = manager.addWorkspace(workingDirectory: "/tmp/project", select: true) + second.setCustomTitle("Second") + XCTAssertEqual(manager.tabs.count, 2, "both workspaces share the same repo path") + + let child = manager.addWorktreeWorkspace( + path: "/tmp/worktrees/feature", branch: "feature", repoRoot: "/tmp/project", + parentWorkspaceId: second.id, select: true + ) + child.setCustomTitle("Child") + XCTAssertEqual(child.worktreeParentWorkspaceId, second.id) + + let encoded = try JSONEncoder().encode(manager.sessionSnapshot(includeScrollback: false)) + let snapshot = try JSONDecoder().decode(SessionTabManagerSnapshot.self, from: encoded) + let restored = TabManager() + restored.restoreSessionSnapshot(snapshot) + + let restoredFirst = try XCTUnwrap(restored.tabs.first { $0.customTitle == "First" }) + let restoredSecond = try XCTUnwrap(restored.tabs.first { $0.customTitle == "Second" }) + let restoredChild = try XCTUnwrap(restored.tabs.first { $0.customTitle == "Child" }) + + XCTAssertEqual( + restoredChild.worktreeParentWorkspaceId, restoredSecond.id, + "the child must attach to the specific workspace recorded as its owner" + ) + XCTAssertNotEqual( + restoredChild.worktreeParentWorkspaceId, restoredFirst.id, + "a same-path sibling that is not the recorded owner must never be substituted" + ) + XCTAssertTrue(restored.worktreeChildren(of: restoredFirst).isEmpty) + XCTAssertEqual(restored.worktreeChildren(of: restoredSecond).map(\.id), [restoredChild.id]) + } + + /// Regression for M9 hierarchy restoration: a folder with two children plus an independent + /// ordinary parent/child/grandchild chain must all survive encode -> decode -> restore + /// together, without cross-linking. `testOrdinaryWorktreeParentSurvivesEncodedSessionRoundTrip` + /// only exercises a single flat parent/child pair, which a restore that merges every worktree + /// under one bucket (or only rebuilds folder membership, the pre-#336 behavior) could still + /// pass. This adds a second, disjoint hierarchy and a selection assertion + /// (`TabManager+SessionPersistence.swift`'s collapsed-parent selection remap) that only holds + /// if the folder/child links were actually rebuilt, not merely present as flat rows. + func testWorkspaceHierarchySurvivesEncodedSessionRoundTrip() throws { + let manager = TabManager() + let folder = try XCTUnwrap(manager.selectedWorkspace) + folder.currentDirectory = "/tmp/folder-repo" + folder.setCustomTitle("Folder") + manager.enableWorktreeFolder(folder, repoRoot: "/tmp/folder-repo") + let folderChildA = manager.addWorktreeWorkspace( + path: "/tmp/worktrees/folder-a", branch: "folder-a", repoRoot: "/tmp/folder-repo", + parentWorkspaceId: folder.id, select: true + ) + folderChildA.setCustomTitle("FolderChildA") + let folderChildB = manager.addWorktreeWorkspace( + path: "/tmp/worktrees/folder-b", branch: "folder-b", repoRoot: "/tmp/folder-repo", + parentWorkspaceId: folder.id, select: true + ) + folderChildB.setCustomTitle("FolderChildB") + manager.setWorktreeFolderCollapsed(folder, collapsed: true) + + let ordinaryParent = manager.addWorkspace(workingDirectory: "/tmp/other-repo", select: true) + ordinaryParent.setCustomTitle("OrdinaryParent") + let ordinaryChild = manager.addWorktreeWorkspace( + path: "/tmp/worktrees/other-child", branch: "other-child", repoRoot: "/tmp/other-repo", + parentWorkspaceId: ordinaryParent.id, select: true + ) + ordinaryChild.setCustomTitle("OrdinaryChild") + let grandchild = manager.addWorktreeWorkspace( + path: "/tmp/worktrees/other-grandchild", branch: "other-grandchild", repoRoot: "/tmp/other-repo", + parentWorkspaceId: ordinaryChild.id, select: true + ) + grandchild.setCustomTitle("Grandchild") + + // Select a folder child so the collapsed-parent selection remap has something to prove. + manager.selectWorkspace(folderChildA) + + let encoded = try JSONEncoder().encode(manager.sessionSnapshot(includeScrollback: false)) + let snapshot = try JSONDecoder().decode(SessionTabManagerSnapshot.self, from: encoded) + let restored = TabManager() + restored.restoreSessionSnapshot(snapshot) + + func tab(_ title: String) throws -> Workspace { + try XCTUnwrap(restored.tabs.first { $0.customTitle == title }) + } + let restoredFolder = try tab("Folder") + let restoredFolderChildA = try tab("FolderChildA") + let restoredFolderChildB = try tab("FolderChildB") + let restoredOrdinaryParent = try tab("OrdinaryParent") + let restoredOrdinaryChild = try tab("OrdinaryChild") + let restoredGrandchild = try tab("Grandchild") + + XCTAssertTrue(restoredFolder.isWorktreeFolder) + XCTAssertTrue(restoredFolder.isWorktreeFolderCollapsed, "collapsed state must survive the round trip") + XCTAssertEqual( + Set(restored.worktreeChildren(of: restoredFolder).map(\.id)), + Set([restoredFolderChildA.id, restoredFolderChildB.id]), + "both folder children must reattach to the folder, and only to the folder" + ) + XCTAssertEqual(restoredFolderChildA.worktreeFolderId, restoredFolder.worktreeFolderId) + XCTAssertEqual(restoredFolderChildB.worktreeFolderId, restoredFolder.worktreeFolderId) + + XCTAssertEqual( + restored.worktreeChildren(of: restoredOrdinaryParent).map(\.id), [restoredOrdinaryChild.id], + "the ordinary (non-folder) hierarchy must stay disjoint from the folder hierarchy" + ) + XCTAssertEqual( + restored.worktreeChildren(of: restoredOrdinaryChild).map(\.id), [restoredGrandchild.id], + "a two-level ordinary chain must preserve both links, not just the first level" + ) + XCTAssertNil( + restoredGrandchild.worktreeFolderId, + "a grandchild under a non-folder parent must never inherit folder membership" + ) + + // The pre-selected folder child's selection remaps to its collapsed folder only if the + // folder/child link the remap depends on was actually rebuilt during restore. + XCTAssertEqual( + restored.selectedTabId, restoredFolder.id, + "selecting a child of a still-collapsed folder must remap to the folder after restore" + ) + } + func testLegacyFolderMembershipRestoresWithoutSavedWorkspaceIds() throws { let manager = TabManager() let folder = try XCTUnwrap(manager.selectedWorkspace) From 18717248d6dee394b15efc5db766d62d241f672d Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 10:59:53 -0300 Subject: [PATCH 03/19] fix: stop notification popover menu and layout churn --- Sources/Update/UpdateTitlebarAccessory.swift | 103 +++++++++++++++++-- 1 file changed, 92 insertions(+), 11 deletions(-) diff --git a/Sources/Update/UpdateTitlebarAccessory.swift b/Sources/Update/UpdateTitlebarAccessory.swift index 58373f5ff..deae936b6 100644 --- a/Sources/Update/UpdateTitlebarAccessory.swift +++ b/Sources/Update/UpdateTitlebarAccessory.swift @@ -1056,6 +1056,11 @@ final class TitlebarControlsAccessoryViewController: NSTitlebarAccessoryViewCont } ) ) + // Popover content must not observe safe-area changes: SwiftUI's observation + // recurses through invalidateSafeAreaInsets on ambient display events until + // AppKit's layout-loop guard kills the app (issue #307/#308 — the opt-out does + // not cascade to sibling hosts, so every new hosting controller needs its own). + hostingController.safeAreaRegions = [] hostingController.view.wantsLayer = true hostingController.view.layer?.backgroundColor = .clear notificationsPopover.contentViewController = hostingController @@ -1295,17 +1300,18 @@ private struct NotificationPopoverRow: View { // XCUITest's `.click()` is not always reliable for SwiftUI `Button`s hosted in an `NSPopover`. // Provide an explicit accessibility action so AXPress always routes to `onOpen`. .accessibilityAction { onOpen() } - .contextMenu { - if notification.isRead { - Button(String(localized: "notificationsPopover.markUnread", defaultValue: "Mark as Unread")) { - onMarkUnread() - } - } else { - Button(String(localized: "notificationsPopover.markRead", defaultValue: "Mark as Read")) { - onMarkRead() - } - } - } + // A SwiftUI `.contextMenu` here hangs off `isRowHovering`/`isRowFocused` + // state: `onHover(false)` fires as the pointer moves from the row onto the + // open menu, re-evaluating this view's body and rebuilding the menu, which + // macOS then closes out from under the click. A native NSMenu shown from a + // right-mouse-down/ctrl-click catcher is immune to that re-evaluation. + .background( + NotificationRowContextMenuCatcher( + isRead: notification.isRead, + onMarkRead: onMarkRead, + onMarkUnread: onMarkUnread + ) + ) Button(action: onClear) { Image(systemName: "xmark.circle.fill") @@ -1333,6 +1339,81 @@ private struct NotificationPopoverRow: View { } } +/// Right-click catcher backing `NotificationPopoverRow`'s context menu. Sized to match +/// the row via `.background(...)`; only intercepts right-click / ctrl-click — every +/// other event falls through to the SwiftUI button/hover handling underneath. +private struct NotificationRowContextMenuCatcher: NSViewRepresentable { + let isRead: Bool + let onMarkRead: () -> Void + let onMarkUnread: () -> Void + + func makeNSView(context: Context) -> NotificationRowContextMenuNSView { + NotificationRowContextMenuNSView() + } + + func updateNSView(_ nsView: NotificationRowContextMenuNSView, context: Context) { + nsView.isRead = isRead + nsView.onMarkRead = onMarkRead + nsView.onMarkUnread = onMarkUnread + } +} + +private final class NotificationRowContextMenuNSView: NSView { + var isRead = false + var onMarkRead: (() -> Void)? + var onMarkUnread: (() -> Void)? + + override var mouseDownCanMoveWindow: Bool { false } + + override func hitTest(_ point: NSPoint) -> NSView? { + // Only intercept right-click and ctrl-click (left-click with the control key + // held, which AppKit does not always redirect to rightMouseDown on its own) so + // plain left-click, hover, and keyboard focus continue to hit-test through to + // the SwiftUI button underneath. + guard let event = NSApp.currentEvent else { return nil } + let isCtrlClick = event.type == .leftMouseDown && event.modifierFlags.contains(.control) + guard event.type == .rightMouseDown || isCtrlClick else { return nil } + return super.hitTest(point) + } + + override func mouseDown(with event: NSEvent) { + guard event.modifierFlags.contains(.control) else { + super.mouseDown(with: event) + return + } + showContextMenu(for: event) + } + + override func rightMouseDown(with event: NSEvent) { + showContextMenu(for: event) + } + + private func showContextMenu(for event: NSEvent) { + let menu = NSMenu() + if isRead { + let item = NSMenuItem( + title: String(localized: "notificationsPopover.markUnread", defaultValue: "Mark as Unread"), + action: #selector(markUnreadAction), + keyEquivalent: "" + ) + item.target = self + menu.addItem(item) + } else { + let item = NSMenuItem( + title: String(localized: "notificationsPopover.markRead", defaultValue: "Mark as Read"), + action: #selector(markReadAction), + keyEquivalent: "" + ) + item.target = self + menu.addItem(item) + } + NSMenu.popUpContextMenu(menu, with: event, for: self) + } + + @objc private func markReadAction() { onMarkRead?() } + @objc private func markUnreadAction() { onMarkUnread?() } +} + @MainActor final class UpdateTitlebarAccessoryController { private weak var updateViewModel: UpdateViewModel? From fdb79b15d7b7534b868bdefec820db95b99e0ceb Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:01:32 -0300 Subject: [PATCH 04/19] fix: send the caller surface with programa notify --- CLI/programa.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CLI/programa.swift b/CLI/programa.swift index 8626c2101..c1563e8ad 100644 --- a/CLI/programa.swift +++ b/CLI/programa.swift @@ -3243,7 +3243,11 @@ struct ProgramaCLI { : (ctx.windowId == nil ? ProcessInfo.processInfo.environment["PROGRAMA_WORKSPACE_ID"] : nil) let workspaceArg = explicitWorkspaceArg ?? callerWorkspaceArg let explicitSurfaceArg = self.optionValue(ctx.commandArgs, name: "--surface") - let callerSurfaceArg = explicitSurfaceArg == nil && workspaceArg == nil && preferTTYFallback == false && ctx.windowId == nil + let envWorkspaceArg = ProcessInfo.processInfo.environment["PROGRAMA_WORKSPACE_ID"] + let callerSurfaceArg = explicitSurfaceArg == nil + && preferTTYFallback == false + && ctx.windowId == nil + && (explicitWorkspaceArg == nil || explicitWorkspaceArg == envWorkspaceArg) ? ProcessInfo.processInfo.environment["PROGRAMA_SURFACE_ID"] : nil let surfaceArg = explicitSurfaceArg ?? callerSurfaceArg From a5a1a1e3f860fc76ae1f04de4f6ff82559dfd494 Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:01:36 -0300 Subject: [PATCH 05/19] refactor: use generated v2 method names in browser commands --- CLI/CLI+Browser.swift | 106 ++++++++++++++++++++++++------------------ 1 file changed, 62 insertions(+), 44 deletions(-) diff --git a/CLI/CLI+Browser.swift b/CLI/CLI+Browser.swift index cd40fb796..9c00a2fd7 100644 --- a/CLI/CLI+Browser.swift +++ b/CLI/CLI+Browser.swift @@ -290,9 +290,9 @@ extension ProgramaCLI { if subcommand == "back" || subcommand == "forward" || subcommand == "reload" { let sid = try requireSurface() let methodMap: [String: String] = [ - "back": "browser.back", - "forward": "browser.forward", - "reload": "browser.reload", + "back": V2MethodNames.browserBack, + "forward": V2MethodNames.browserForward, + "reload": V2MethodNames.browserReload, ] var params: [String: Any] = ["surface_id": sid] if hasFlag(subArgs, name: "--snapshot-after") { @@ -437,15 +437,15 @@ extension ProgramaCLI { throw CLIError(message: "browser \(subcommand) requires a selector") } let methodMap: [String: String] = [ - "click": "browser.click", - "dblclick": "browser.dblclick", - "hover": "browser.hover", - "focus": "browser.focus", - "check": "browser.check", - "uncheck": "browser.uncheck", - "scrollintoview": "browser.scroll_into_view", - "scrollinto": "browser.scroll_into_view", - "scroll-into-view": "browser.scroll_into_view", + "click": V2MethodNames.browserClick, + "dblclick": V2MethodNames.browserDblclick, + "hover": V2MethodNames.browserHover, + "focus": V2MethodNames.browserFocus, + "check": V2MethodNames.browserCheck, + "uncheck": V2MethodNames.browserUncheck, + "scrollintoview": V2MethodNames.browserScrollIntoView, + "scrollinto": V2MethodNames.browserScrollIntoView, + "scroll-into-view": V2MethodNames.browserScrollIntoView, ] var params: [String: Any] = ["surface_id": sid, "selector": selector] if hasFlag(subArgs, name: "--snapshot-after") { @@ -504,7 +504,7 @@ extension ProgramaCLI { } } - let method = (subcommand == "type") ? "browser.type" : "browser.fill" + let method = (subcommand == "type") ? V2MethodNames.browserType : V2MethodNames.browserFill var params: [String: Any] = ["surface_id": sid, "selector": selector, "text": text] if snapshotAfter { params["snapshot_after"] = true @@ -522,10 +522,10 @@ extension ProgramaCLI { throw CLIError(message: "browser \(subcommand) requires a key") } let methodMap: [String: String] = [ - "press": "browser.press", - "key": "browser.press", - "keydown": "browser.keydown", - "keyup": "browser.keyup", + "press": V2MethodNames.browserPress, + "key": V2MethodNames.browserPress, + "keydown": V2MethodNames.browserKeydown, + "keyup": V2MethodNames.browserKeyup, ] var params: [String: Any] = ["surface_id": sid, "key": key] if hasFlag(subArgs, name: "--snapshot-after") { @@ -775,13 +775,13 @@ extension ProgramaCLI { } let methodMap: [String: String] = [ - "text": "browser.get.text", - "html": "browser.get.html", - "value": "browser.get.value", - "attr": "browser.get.attr", - "count": "browser.get.count", - "box": "browser.get.box", - "styles": "browser.get.styles", + "text": V2MethodNames.browserGetText, + "html": V2MethodNames.browserGetHtml, + "value": V2MethodNames.browserGetValue, + "attr": V2MethodNames.browserGetAttr, + "count": V2MethodNames.browserGetCount, + "box": V2MethodNames.browserGetBox, + "styles": V2MethodNames.browserGetStyles, ] let payload = try client.sendV2(method: methodMap[getVerb]!, params: params) if effectiveJSONOutput { @@ -816,9 +816,9 @@ extension ProgramaCLI { } let methodMap: [String: String] = [ - "visible": "browser.is.visible", - "enabled": "browser.is.enabled", - "checked": "browser.is.checked", + "visible": V2MethodNames.browserIsVisible, + "enabled": V2MethodNames.browserIsEnabled, + "checked": V2MethodNames.browserIsChecked, ] guard let method = methodMap[isVerb] else { throw CLIError(message: "Unsupported browser is subcommand: \(isVerb)") @@ -859,7 +859,7 @@ extension ProgramaCLI { if hasFlag(locatorArgs, name: "--exact") { params["exact"] = true } - method = "browser.find.role" + method = V2MethodNames.browserFindRole case "text", "label", "placeholder", "alt", "title", "testid": let keyMap: [String: String] = [ "text": "text", @@ -869,6 +869,14 @@ extension ProgramaCLI { "title": "title", "testid": "testid", ] + let findMethodMap: [String: String] = [ + "text": V2MethodNames.browserFindText, + "label": V2MethodNames.browserFindLabel, + "placeholder": V2MethodNames.browserFindPlaceholder, + "alt": V2MethodNames.browserFindAlt, + "title": V2MethodNames.browserFindTitle, + "testid": V2MethodNames.browserFindTestid, + ] let candidates = nonFlagArgs(locatorArgs) guard let value = candidates.first else { throw CLIError(message: "browser find \(locator) requires a value") @@ -877,7 +885,7 @@ extension ProgramaCLI { if hasFlag(locatorArgs, name: "--exact") { params["exact"] = true } - method = "browser.find.\(locator)" + method = findMethodMap[locator]! case "first", "last": let (selectorOpt, rem1) = parseOption(locatorArgs, name: "--selector") let candidates = nonFlagArgs(rem1) @@ -885,7 +893,7 @@ extension ProgramaCLI { throw CLIError(message: "browser find \(locator) requires a selector") } params["selector"] = selector - method = "browser.find.\(locator)" + method = (locator == "first") ? V2MethodNames.browserFindFirst : V2MethodNames.browserFindLast case "nth": let (indexOpt, rem1) = parseOption(locatorArgs, name: "--index") let (selectorOpt, rem2) = parseOption(rem1, name: "--selector") @@ -901,7 +909,7 @@ extension ProgramaCLI { } params["index"] = index params["selector"] = selector - method = "browser.find.nth" + method = V2MethodNames.browserFindNth default: throw CLIError(message: "Unsupported browser find locator: \(locator)") } @@ -1114,7 +1122,7 @@ extension ProgramaCLI { let payload = try client.sendV2(method: V2MethodNames.browserTabNew, params: params) output(payload, fallback: "OK") case "switch", "close": - let method = (tabVerb == "switch") ? "browser.tab.switch" : "browser.tab.close" + let method = (tabVerb == "switch") ? V2MethodNames.browserTabSwitch : V2MethodNames.browserTabClose var params: [String: Any] = ["surface_id": sid] let target = tabArgs.first if let target { @@ -1135,7 +1143,7 @@ extension ProgramaCLI { if subcommand == "console" { let sid = try requireSurface() let consoleVerb = subArgs.first?.lowercased() ?? "list" - let method = (consoleVerb == "clear") ? "browser.console.clear" : "browser.console.list" + let method = (consoleVerb == "clear") ? V2MethodNames.browserConsoleClear : V2MethodNames.browserConsoleList if consoleVerb != "list" && consoleVerb != "clear" { throw CLIError(message: "Unsupported browser console subcommand: \(consoleVerb)") } @@ -1190,9 +1198,9 @@ extension ProgramaCLI { let method: String switch stateVerb { case "save": - method = "browser.state.save" + method = V2MethodNames.browserStateSave case "load": - method = "browser.state.load" + method = V2MethodNames.browserStateLoad default: throw CLIError(message: "Unsupported browser state subcommand: \(stateVerb)") } @@ -1210,7 +1218,12 @@ extension ProgramaCLI { guard !content.isEmpty else { throw CLIError(message: "browser \(subcommand) requires content") } - let payload = try client.sendV2(method: "browser.\(subcommand)", params: ["surface_id": sid, field: content]) + let addMethodMap: [String: String] = [ + "addinitscript": V2MethodNames.browserAddinitscript, + "addscript": V2MethodNames.browserAddscript, + "addstyle": V2MethodNames.browserAddstyle, + ] + let payload = try client.sendV2(method: addMethodMap[subcommand]!, params: ["surface_id": sid, field: content]) output(payload, fallback: "OK") return } @@ -1258,9 +1271,9 @@ extension ProgramaCLI { let method: String switch traceVerb { case "start": - method = "browser.trace.start" + method = V2MethodNames.browserTraceStart case "stop": - method = "browser.trace.stop" + method = V2MethodNames.browserTraceStop default: throw CLIError(message: "Unsupported browser trace subcommand: \(traceVerb)") } @@ -1317,9 +1330,9 @@ extension ProgramaCLI { let method: String switch castVerb { case "start": - method = "browser.screencast.start" + method = V2MethodNames.browserScreencastStart case "stop": - method = "browser.screencast.stop" + method = V2MethodNames.browserScreencastStop default: throw CLIError(message: "Unsupported browser screencast subcommand: \(castVerb)") } @@ -1337,11 +1350,11 @@ extension ProgramaCLI { let method: String switch inputVerb { case "mouse": - method = "browser.input_mouse" + method = V2MethodNames.browserInputMouse case "keyboard": - method = "browser.input_keyboard" + method = V2MethodNames.browserInputKeyboard case "touch": - method = "browser.input_touch" + method = V2MethodNames.browserInputTouch default: throw CLIError(message: "Unsupported browser input subcommand: \(inputVerb)") } @@ -1356,7 +1369,12 @@ extension ProgramaCLI { if ["input_mouse", "input_keyboard", "input_touch"].contains(subcommand) { let sid = try requireSurface() - let payload = try client.sendV2(method: "browser.\(subcommand)", params: ["surface_id": sid]) + let legacyInputMethodMap: [String: String] = [ + "input_mouse": V2MethodNames.browserInputMouse, + "input_keyboard": V2MethodNames.browserInputKeyboard, + "input_touch": V2MethodNames.browserInputTouch, + ] + let payload = try client.sendV2(method: legacyInputMethodMap[subcommand]!, params: ["surface_id": sid]) output(payload, fallback: "OK") return } From 008e1b38de20cdefd5785ad7654f7d57ebeb883c Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:01:43 -0300 Subject: [PATCH 06/19] test(programad): add regression test for session.close SIGKILL escalation Covers M5 from docs/audits/codebase-audit-2026-09-28.md: a session whose child ignores SIGHUP must still be reaped by session.close(kill: true) instead of returning Timeout and leaving the process running. Currently red against the unfixed close path (no SIGKILL escalation). --- core/crates/programad/tests/integration.rs | 81 ++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/core/crates/programad/tests/integration.rs b/core/crates/programad/tests/integration.rs index 1fac30f10..6cdf5e430 100644 --- a/core/crates/programad/tests/integration.rs +++ b/core/crates/programad/tests/integration.rs @@ -515,3 +515,84 @@ async fn workspace_dispatch_links_domain_surfaces_to_sessions_and_reconciles_on_ let _ = shutdown_tx.send(()); let _ = tokio::time::timeout(Duration::from_secs(5), daemon_handle).await; } + +/// `session.close(kill: true)` against a child that ignores SIGHUP must +/// still escalate to SIGKILL and reap it, rather than returning `Timeout` +/// and leaving the process running (M5, `docs/audits/codebase-audit-2026-09-28.md`). +/// The whole round trip — SIGHUP grace period, SIGKILL escalation, reap — +/// happens off the async worker (`spawn_blocking` in `server.rs`), so this +/// also proves that path doesn't hang or error on a stubborn child. +#[tokio::test] +async fn session_close_escalates_to_sigkill_when_child_ignores_sighup() { + let _env_guard = ENV_LOCK.lock().unwrap(); + let dir = tempfile::tempdir().unwrap(); + let socket_path = dir.path().join("programad.sock"); + std::env::set_var("XDG_STATE_HOME", dir.path()); + + let (daemon_handle, shutdown_tx) = spawn_daemon(socket_path.clone()); + let mut client = Client::connect(&socket_path).await; + client.call("auth.login", json!({})).await; + + let opened = client + .call( + "session.open", + json!({ + "argv": ["/bin/sh", "-c", "trap '' HUP; echo READY; sleep 60"], + "cols": 80, + "rows": 24, + }), + ) + .await; + let session_id = opened["id"].as_str().unwrap().to_string(); + let pid = opened["pid"].as_i64().unwrap() as i32; + + // Wait for the shell to actually install the trap and print READY (via + // the WAL, same synchronization pattern as the MARK_ wait above) before + // closing. Without this, SIGHUP could land before `trap` runs and reap + // the child the "easy" way, which wouldn't exercise the escalation path + // this test exists to cover. + let mut saw_ready = false; + for _ in 0..100 { + let read = client + .call("session.read", json!({"id": session_id, "offset": 0})) + .await; + let chunk = B64.decode(read["data"].as_str().unwrap()).unwrap(); + if chunk.windows(5).any(|w| w == b"READY") { + saw_ready = true; + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + assert!(saw_ready, "shell never printed READY after installing its SIGHUP trap"); + + // Expected cost is ~500ms SIGHUP grace + ~500ms SIGKILL grace plus reap/ + // join/flush overhead; give real headroom above that rather than a tight + // bound, since the two assertions below (closed: true, then ESRCH) are + // what actually discriminate pass/fail, not this timeout. + let closed = tokio::time::timeout( + Duration::from_secs(3), + client.call("session.close", json!({"id": session_id, "kill": true})), + ) + .await + .expect("session.close should not hang past its own SIGHUP+SIGKILL grace periods"); + assert_eq!(closed, json!({"closed": true})); + + // The process must actually be gone (not just marked closed + // server-side): `kill(pid, 0)` on a reaped pid returns ESRCH. + for _ in 0..50 { + if nix::sys::signal::kill(nix::unistd::Pid::from_raw(pid), None) + == Err(nix::errno::Errno::ESRCH) + { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + assert_eq!( + nix::sys::signal::kill(nix::unistd::Pid::from_raw(pid), None), + Err(nix::errno::Errno::ESRCH), + "child that ignores SIGHUP must be SIGKILLed and reaped by session.close" + ); + + let _ = shutdown_tx.send(()); + let _ = tokio::time::timeout(Duration::from_secs(5), daemon_handle).await; +} From 93b6a8b3960a9492988a579dfc1338e1556354fc Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:01:51 -0300 Subject: [PATCH 07/19] fix(programad): reap closed sessions off the async worker and escalate to SIGKILL --- core/crates/programad/src/server.rs | 40 +++++++++++------- core/crates/programad/src/session.rs | 61 +++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 25 deletions(-) diff --git a/core/crates/programad/src/server.rs b/core/crates/programad/src/server.rs index 4882fd584..1ac78cdf0 100644 --- a/core/crates/programad/src/server.rs +++ b/core/crates/programad/src/server.rs @@ -153,7 +153,7 @@ pub async fn handle_connection(stream: UnixStream, state: Arc) { continue; } - let outcome = dispatch(&request, &state, &mut authenticated, &mut my_attachments); + let outcome = dispatch(&request, &state, &mut authenticated, &mut my_attachments).await; let (resp_line, fd_to_send, pending_attachment) = match outcome { Ok(Dispatched { @@ -202,7 +202,7 @@ fn rollback_attachment(state: &AppState, attachment: Option<(String, Uuid)>) { } } -fn dispatch( +async fn dispatch( req: &Request, state: &Arc, authenticated: &mut bool, @@ -221,7 +221,7 @@ fn dispatch( "session.list" => session_list(state).map(Into::into), "session.status" => session_status(req, state).map(Into::into), "session.resize" => session_resize(req, state, attachments).map(Into::into), - "session.close" => session_close(req, state).map(Into::into), + "session.close" => session_close(req, state).await.map(Into::into), "session.write" => session_write(req, state).map(Into::into), "session.read" => session_read(req, state).map(Into::into), "session.detach" => session_detach(req, state, attachments).map(Into::into), @@ -447,8 +447,13 @@ fn session_resize( Ok(json!({"cols": eff_cols, "rows": eff_rows})) } -fn session_close(req: &Request, state: &Arc) -> Result { - let id = require_str(&req.params, "id")?; +// `Session::close` blocks on SIGHUP/SIGKILL grace-period polling (up to ~1s, +// see session.rs), so it runs on a blocking-pool thread via `spawn_blocking` +// rather than inline on this async connection's tokio worker — otherwise +// closing one session would stall every other connection sharing the +// runtime for the duration of that wait. +async fn session_close(req: &Request, state: &Arc) -> Result { + let id = require_str(&req.params, "id")?.to_string(); let kill = req .params .get("kill") @@ -460,18 +465,25 @@ fn session_close(req: &Request, state: &Arc) -> Result, wake_raw: RawFd) { fn fail_session(inner: &Arc, error: String) { *inner.status.lock().unwrap() = SessionStatus::Failed { error }; - let _ = signal_child(inner, nix::sys::signal::Signal::SIGHUP); + let _ = signal_child(inner, nix::sys::signal::Signal::SIGHUP, false); } fn try_reap(inner: &Inner) -> io::Result, Option)>> { @@ -579,9 +582,33 @@ fn try_reap(inner: &Inner) -> io::Result, Option)>> { } } +/// Poll `try_reap` at a fixed interval until the child exits or `timeout` +/// elapses. Runs on whatever thread calls `Session::close`; callers on an +/// async runtime must offload that call (e.g. `spawn_blocking`) so this +/// sleep never blocks a tokio worker. +fn wait_for_exit( + inner: &Inner, + mut exit: Option<(Option, Option)>, + timeout: Duration, +) -> io::Result, Option)>> { + let deadline = Instant::now() + timeout; + while exit.is_none() && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(25)); + exit = try_reap(inner)?; + } + Ok(exit) +} + +/// Signal the child, or (for the SIGKILL escalation once a plain SIGHUP has +/// been given a grace period and ignored) its whole process group — +/// `whole_group: true` uses `killpg` instead of `kill`. Signaling the group +/// is safe because the child calls `setsid()` at spawn (see `pty.rs`), so +/// its pgid equals its own pid and this can never reach the daemon's own +/// process group. fn signal_child( inner: &Inner, signal: nix::sys::signal::Signal, + whole_group: bool, ) -> io::Result, Option)>> { let mut child = inner.child.lock().unwrap(); if let Some(exit) = child.exit { @@ -596,7 +623,19 @@ fn signal_child( // The child lock stays held across the liveness check and signal. No // reader can reap it in between, so its PID cannot be recycled here. let pid = Pid::from_raw(child.child.id() as i32); - match nix::sys::signal::kill(pid, signal) { + let result = if whole_group { + nix::sys::signal::killpg(pid, signal) + } else { + nix::sys::signal::kill(pid, signal) + }; + let result = match result { + // The child already left its own group (e.g. called setpgid), so + // the group no longer exists even though the pid might still be + // alive. Fall back to signaling the pid directly before giving up. + Err(nix::errno::Errno::ESRCH) if whole_group => nix::sys::signal::kill(pid, signal), + other => other, + }; + match result { Ok(()) => Ok(None), Err(nix::errno::Errno::ESRCH) => { if let Some(status) = child.child.try_wait()? { From dc5b8e14e8d18f03ca3d404b37c94e0cc6adf639 Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:06:55 -0300 Subject: [PATCH 08/19] fix: make socket client handling nonisolated --- Sources/TerminalController.swift | 34 +++++++++++++++++++------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 61daa2b5f..1a843a4c1 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -157,7 +157,7 @@ class TerminalController { case failure(path: String, stage: String, errnoCode: Int32) } - private static let focusIntentV2Methods: Set = [ + private nonisolated static let focusIntentV2Methods: Set = [ "window.focus", "workspace.select", "review.open", @@ -578,7 +578,7 @@ class TerminalController { } } - private static func socketCommandAllowsInAppFocusMutations(commandKey: String, isV2: Bool) -> Bool { + private nonisolated static func socketCommandAllowsInAppFocusMutations(commandKey: String, isV2: Bool) -> Bool { // The v1 line protocol is gone; only v2 JSON-RPC methods can carry focus intent. guard isV2 else { return false } return focusIntentV2Methods.contains(commandKey) @@ -601,7 +601,7 @@ class TerminalController { return body() } - private func withSocketCommandPolicy(commandKey: String, isV2: Bool, _ body: () -> T) -> T { + private nonisolated func withSocketCommandPolicy(commandKey: String, isV2: Bool, _ body: () -> T) -> T { let allowsFocusMutation = Self.socketCommandAllowsInAppFocusMutations(commandKey: commandKey, isV2: isV2) Self.socketCommandPolicyLock.lock() Self.socketCommandPolicyDepth += 1 @@ -940,7 +940,7 @@ class TerminalController { } /// Check if `pid` is a descendant of this process by walking the process tree. - func isDescendant(_ pid: pid_t) -> Bool { + nonisolated func isDescendant(_ pid: pid_t) -> Bool { var current = pid // Walk up to 128 levels to avoid infinite loops from kernel bugs for _ in 0..<128 { @@ -960,7 +960,7 @@ class TerminalController { } /// Get the parent PID of a process using sysctl. - private func parentPid(of pid: pid_t) -> pid_t { + private nonisolated func parentPid(of pid: pid_t) -> pid_t { var info = kinfo_proc() var size = MemoryLayout.size var mib: [Int32] = [CTL_KERN, KERN_PROC, KERN_PROC_PID, pid] @@ -1489,7 +1489,7 @@ class TerminalController { } - private func passwordAuthRequiredResponse(for command: String) -> String { + private nonisolated func passwordAuthRequiredResponse(for command: String) -> String { let message = "Authentication required. Send auth first." guard command.hasPrefix("{"), let data = command.data(using: .utf8), @@ -1500,7 +1500,7 @@ class TerminalController { return v2Error(id: id, code: "auth_required", message: message) } - private func passwordLoginV2ResponseIfNeeded(for command: String, authenticated: inout Bool) -> String? { + private nonisolated func passwordLoginV2ResponseIfNeeded(for command: String, authenticated: inout Bool) -> String? { guard command.hasPrefix("{"), let data = command.data(using: .utf8), let dict = (try? JSONSerialization.jsonObject(with: data, options: [])) as? [String: Any] else { @@ -1536,7 +1536,7 @@ class TerminalController { return v2Ok(id: id, result: ["authenticated": true]) } - private func authResponseIfNeeded( + private nonisolated func authResponseIfNeeded( for command: String, authenticated: inout Bool, requestPolicy: SocketRequestPolicy @@ -1760,7 +1760,7 @@ class TerminalController { /// Unix clients carry the listener policy captured when they were accepted. /// Mobile Bridge sessions are admitted independently by pairing and its method allow-list. - func handleClient(_ socket: Int32, peerPid: pid_t? = nil, source: SocketConnectionSource) { + nonisolated func handleClient(_ socket: Int32, peerPid: pid_t? = nil, source: SocketConnectionSource) { // Owns this connection's writes (both ordinary v2 responses and any #167 subscription // event pushes) and its subscription lifecycle. `teardown()` (which tears down any // attached subscription) must run before the fd is closed -- defers unwind LIFO, so the @@ -1889,7 +1889,7 @@ class TerminalController { } } - private func processCommand( + private nonisolated func processCommand( _ command: String, connection: SocketConnection, requestPolicy: SocketRequestPolicy @@ -1913,7 +1913,7 @@ class TerminalController { // MARK: - V2 JSON Socket Protocol - private func processV2Command( + private nonisolated func processV2Command( _ jsonLine: String, connection: SocketConnection, requestPolicy: SocketRequestPolicy @@ -1953,7 +1953,7 @@ class TerminalController { } return withSocketCommandPolicy(commandKey: method, isV2: true) { - if let result = browserRPCDispatcher.dispatch(method: method, params: params, controller: self) ?? AgentRPCDispatcher.dispatch(method: method, params: params, controller: self) { + if let result = AgentRPCDispatcher.dispatch(method: method, params: params, controller: self) { return v2Result(id: id, result) } switch method { @@ -2195,7 +2195,8 @@ class TerminalController { // Markdown case "markdown.open": - return v2Result(id: id, self.v2MarkdownOpen(params: params)) + // `v2MarkdownOpen` is main actor-isolated; hop explicitly. + return v2Result(id: id, v2MainSync { self.v2MarkdownOpen(params: params) }) // Review (agent diff review panel) case "review.open": @@ -2297,12 +2298,17 @@ class TerminalController { #endif default: + // BrowserRPCDispatcher is @MainActor; only methods that missed every + // nonisolated case above pay this hop, keeping telemetry off main. + if let result = v2MainSync({ browserRPCDispatcher.dispatch(method: method, params: params, controller: self) }) { + return v2Result(id: id, result) + } return v2Error(id: id, code: "method_not_found", message: "Unknown method") } } } - private func v2Capabilities(requestPolicy: SocketRequestPolicy) -> [String: Any] { + private nonisolated func v2Capabilities(requestPolicy: SocketRequestPolicy) -> [String: Any] { return [ "protocol": "cmux-socket", "version": 2, From e275fb35c9496075509a2f62239c0920a863820b Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:06:55 -0300 Subject: [PATCH 09/19] fix: let replay release the terminal surface --- Sources/TerminalSurface.swift | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Sources/TerminalSurface.swift b/Sources/TerminalSurface.swift index 45f2e3870..4f729fcf2 100644 --- a/Sources/TerminalSurface.swift +++ b/Sources/TerminalSurface.swift @@ -2154,7 +2154,19 @@ final class TerminalSurface: Identifiable, ObservableObject { pendingReviveWinchPGID = nil pendingReviveWinchChildPID = nil - reviveReplayQueue.async { [reviveReplayCanceled] in + // Read into locals up front so the closure below has no reason to reach `self` other + // than the `[weak self]` capture it needs for the main-queue hop at the end -- a `Self.` + // static reference inside the closure would also capture `self` (strongly, to resolve + // the dynamic type), which is a second, redundant path to the same capture. + let reviveReplayChunkBytes = Self.reviveReplayChunkBytes +#if DEBUG + let reviveReplayChunkObserverForTesting = Self.reviveReplayChunkObserverForTesting +#endif + + // `weak self` here (not just on the inner hop) is what actually lets the surface + // deallocate mid-replay: without it the compiler captures `self` strongly by default + // to make it available to the inner closure's own `[weak self]`. + reviveReplayQueue.async { [reviveReplayCanceled, weak self] in // The chunk loop runs to completion here rather than yielding the // run loop between chunks. Releasing the mutex is what breaks the // deadlock; yielding as well would additionally let live output @@ -2165,7 +2177,7 @@ final class TerminalSurface: Identifiable, ObservableObject { var offset = 0 while offset < bytes.count { if reviveReplayCanceled.withLock({ $0 }) { break } - let end = min(offset + Self.reviveReplayChunkBytes, bytes.count) + let end = min(offset + reviveReplayChunkBytes, bytes.count) bytes.withUnsafeBufferPointer { buffer in guard let baseAddress = buffer.baseAddress else { return } UnsafeRawPointer(baseAddress + offset) @@ -2175,7 +2187,7 @@ final class TerminalSurface: Identifiable, ObservableObject { } offset = end #if DEBUG - Self.reviveReplayChunkObserverForTesting?(Thread.isMainThread) + reviveReplayChunkObserverForTesting?(Thread.isMainThread) #endif } From 19a7e2c6d7406dfd1b502f49b4aa7b7bebb06223 Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:03:17 -0300 Subject: [PATCH 10/19] fix: host the sidebar workspace menu in a native NSMenu --- CLAUDE.md | 2 +- Sources/TabItemView.swift | 539 ++++++++++++++++++++++---------------- 2 files changed, 318 insertions(+), 223 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9a1db421e..9b48d3316 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -159,7 +159,7 @@ The app has a **Debug** menu in the macOS menu bar (only in DEBUG builds). Use i - **Typing-latency-sensitive paths** (read carefully before touching these areas): - `WindowTerminalHostView.hitTest()` in `Sources/WindowTerminalHostView.swift`: called on every event including keyboard. Keyboard events (`.keyDown`/`.keyUp`/`.flagsChanged`) take an early `switch currentEvent?.type` fast path; everything else — all pointer events, and an ambiguous/nil `currentEvent` — must fall through to the full divider/sidebar/drag routing below it. Do not add work to that keyboard fast path. - `NSWindow.programa_sendEvent` in `Sources/WindowSwizzles.swift`: runs for every event. The hit-view context it caches is only ever read for pointer-down events, so it is computed only for those — do not restore an unconditional hit-test here (#183). - - `TabItemView` in `ContentView.swift`: uses `Equatable` conformance + `.equatable()` to skip body re-evaluation during typing. Do not add `@EnvironmentObject`, `@ObservedObject` (besides `tab`), or `@Binding` properties without updating the `==` function. Do not remove `.equatable()` from the ForEach call site. Do not read `tabManager` or `notificationStore` in the body; use the precomputed `let` parameters instead. + - `TabItemView` in `Sources/TabItemView.swift`: uses `Equatable` conformance + `.equatable()` to skip body re-evaluation during typing. Do not add `@EnvironmentObject`, `@ObservedObject` (besides `tab`), or `@Binding` properties without updating the `==` function. Do not remove `.equatable()` from the ForEach call site. Do not read `tabManager` or `notificationStore` in the body; use the precomputed `let` parameters instead. - `TerminalSurface.forceRefresh()` in `GhosttyTerminalView.swift`: called on every keystroke. Do not add allocations, file I/O, or formatting here. - **Terminal find layering contract:** `SurfaceSearchOverlay` must be mounted from `GhosttySurfaceScrollView` in `Sources/GhosttyTerminalView.swift` (AppKit portal layer), not from SwiftUI panel containers such as `Sources/Panels/TerminalPanelView.swift`. Portal-hosted terminal views can sit above SwiftUI during split/workspace churn. - **Submodule safety:** When modifying a submodule (ghostty), 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 && git merge-base --is-ancestor HEAD origin/main`. Note: `vendor/bonsplit` is NOT a submodule — it is vendored in-tree (MIT, from the manaflow-ai fork); edit and commit it like any other source directory. diff --git a/Sources/TabItemView.swift b/Sources/TabItemView.swift index b83b69fe1..c166a61ac 100644 --- a/Sources/TabItemView.swift +++ b/Sources/TabItemView.swift @@ -1,11 +1,10 @@ -// TabItemView + its exclusive helper-view cluster, extracted from ContentView.swift (nuclear-review #94.2). -// Pure move — behavior-identical relocation. Do NOT touch TabItemView's Equatable -// conformance, its precomputed let parameters, or the .equatable() call site in -// VerticalTabsSidebar (ContentView.swift) — see CLAUDE.md typing-latency-sensitive paths. +// TabItemView + its exclusive helper-view cluster. Do NOT touch TabItemView's +// Equatable conformance, its precomputed let parameters, or the .equatable() call +// site in VerticalTabsSidebar (Sources/VerticalTabsSidebar.swift) — see CLAUDE.md +// typing-latency-sensitive paths. // -// Access-level widening: TabItemView was `private struct` (file-private to the old -// ContentView.swift); widened to internal (default) so VerticalTabsSidebar, which stays -// in ContentView.swift, can still construct it. No other behavior change. +// TabItemView is internal (not private) so VerticalTabsSidebar, defined in +// Sources/VerticalTabsSidebar.swift, can construct it. import AppKit import Bonsplit @@ -746,6 +745,23 @@ struct TabItemView: View, Equatable { tabManager.closeWorkspaceWithConfirmation(tab) } } + .overlay { + // Native AppKit context menu, built fresh from live state at click time, + // instead of SwiftUI's `.contextMenu`. `.contextMenu`'s content closure + // hangs off this row's body, which re-evaluates as often as every 40ms + // from the onReceive handlers above (`workspaceObservationGeneration`), + // on hover changes, and on any ==-compared param change. SwiftUI rebuilds + // an already-open `.contextMenu` on every one of those re-evaluations, + // and macOS reads that rebuild as the menu (and any open submenu, e.g. + // Workspace Color) repeatedly closing and reopening. Building the NSMenu + // only once, at the moment of the click, sidesteps the churn entirely. + SidebarRowContextMenuCapture { + #if DEBUG + dlog("sidebar.contextMenu.open workspace=\(tab.id.uuidString.prefix(5)) targets=\(contextMenuWorkspaceIds.count)") + #endif + return buildWorkspaceContextMenu(colorScheme: colorScheme) + } + } .overlay(alignment: .top) { if showsCenteredTopDropIndicator { Rectangle() @@ -865,90 +881,34 @@ struct TabItemView: View, Equatable { // Toggling branch/PR columns changes which data we need to cache. recomputeSidebarDetailCache() } - .contextMenu { workspaceContextMenu } } private func contextMenuLabel(multi: String, single: String, isMulti: Bool) -> String { isMulti ? multi : single } - // Isolates the workspace-color submenu from the churning per-row - // observation tick (`workspaceObservationGeneration`, bumped as often as - // every 40ms while workspace telemetry is updating -- see the - // `let _ = workspaceObservationGeneration` line at the top of `body`). - // `.contextMenu`'s content closure is re-invoked on every TabItemView.body - // re-evaluation; without `.equatable()` gating this subtree, AppKit tears - // down and rebuilds the color submenu's NSMenu on each tick, which -- while - // the submenu is open under mouse hover -- reads as the submenu repeatedly - // closing and reopening. Wrapping the submenu in its own Equatable view and - // applying `.equatable()` at the call site lets SwiftUI skip rebuilding it - // when none of its own (non-churning) inputs changed, mirroring the - // TabItemView Equatable contract in CLAUDE.md "Typing-latency-sensitive paths". - private struct WorkspaceColorMenu: View, Equatable { - let hasCustomColor: Bool - let palette: [WorkspaceTabColorEntry] - let targetIds: [UUID] - let colorScheme: ColorScheme - let activeTabIndicatorStyle: SidebarActiveTabIndicatorStyle - let onApplyColor: (String?, [UUID]) -> Void - let onPromptCustomColor: ([UUID]) -> Void - - nonisolated static func == (lhs: Self, rhs: Self) -> Bool { - lhs.hasCustomColor == rhs.hasCustomColor && - lhs.palette == rhs.palette && - lhs.targetIds == rhs.targetIds && - lhs.colorScheme == rhs.colorScheme && - lhs.activeTabIndicatorStyle == rhs.activeTabIndicatorStyle - } - - private func swatchColor(for hex: String) -> NSColor { - WorkspaceTabColorSettings.displayNSColor( - hex: hex, - colorScheme: colorScheme, - forceBright: activeTabIndicatorStyle == .leftRail - ) ?? NSColor(hex: hex) ?? .gray - } - - var body: some View { - Menu(String(localized: "contextMenu.workspaceColor", defaultValue: "Workspace Color")) { - if hasCustomColor { - Button { - onApplyColor(nil, targetIds) - } label: { - Label(String(localized: "contextMenu.clearColor", defaultValue: "Clear Color"), systemImage: "xmark.circle") - } - } - - Button { - onPromptCustomColor(targetIds) - } label: { - Label(String(localized: "contextMenu.chooseCustomColor", defaultValue: "Choose Custom Color…"), systemImage: "paintpalette") - } - - if !palette.isEmpty { - Divider() - } - - ForEach(palette, id: \.id) { entry in - Button { - onApplyColor(entry.hex, targetIds) - } label: { - Label { - Text(entry.name) - } icon: { - Image(nsImage: coloredCircleImage(color: swatchColor(for: entry.hex))) - } - } - } - } - } + /// Applies a `StoredShortcut`'s key equivalent to an `NSMenuItem`, mirroring the + /// `if let key = shortcut.keyEquivalent { .keyboardShortcut(key, modifiers:) }` + /// pattern the SwiftUI menu used to use. `menuItemKeyEquivalent`/`modifierFlags` + /// are the AppKit-side counterparts of `keyEquivalent`/`eventModifiers` and are + /// `nil`/empty under the same conditions (chorded shortcuts have no single + /// menu-item key equivalent). + private func applyShortcut(_ shortcut: StoredShortcut, to item: NSMenuItem) { + guard let key = shortcut.menuItemKeyEquivalent else { return } + item.keyEquivalent = key + item.keyEquivalentModifierMask = shortcut.modifierFlags } - @ViewBuilder - private var workspaceContextMenu: some View { + /// Builds the sidebar row's context menu fresh from live state at click time, + /// replacing the SwiftUI `.contextMenu` this row used to carry. Every item here + /// ports the SwiftUI menu 1:1 (same strings/keys, order, dividers, conditional + /// visibility, disabled states, and shortcut display). + private func buildWorkspaceContextMenu(colorScheme: ColorScheme) -> NSMenu { let targetIds = contextMenuWorkspaceIds let isMulti = targetIds.count > 1 - let tabColorPalette = WorkspaceTabColorSettings.palette() + let menu = NSMenu() + menu.autoenablesItems = false + let shouldPin = !tab.isPinned let pinLabel = shouldPin ? contextMenuLabel( @@ -978,190 +938,210 @@ struct TabItemView: View, Equatable { let renameWorkspaceShortcut = KeyboardShortcutSettings.shortcut(for: .renameWorkspace) let editWorkspaceDescriptionShortcut = KeyboardShortcutSettings.shortcut(for: .editWorkspaceDescription) let closeWorkspaceShortcut = KeyboardShortcutSettings.shortcut(for: .closeWorkspace) - Button(pinLabel) { + + menu.addItem(SidebarMenuItem(title: pinLabel) { [self] in for id in targetIds { if let tab = tabManager.tabs.first(where: { $0.id == id }) { tabManager.setPinned(tab, pinned: shouldPin) } } syncSelectionAfterMutation() - } + }) - if let key = renameWorkspaceShortcut.keyEquivalent { - Button(String(localized: "contextMenu.renameWorkspace", defaultValue: "Rename Workspace…")) { - promptRename() - } - .keyboardShortcut(key, modifiers: renameWorkspaceShortcut.eventModifiers) - } else { - Button(String(localized: "contextMenu.renameWorkspace", defaultValue: "Rename Workspace…")) { - promptRename() - } - } + let renameItem = SidebarMenuItem( + title: String(localized: "contextMenu.renameWorkspace", defaultValue: "Rename Workspace…") + ) { [self] in promptRename() } + applyShortcut(renameWorkspaceShortcut, to: renameItem) + menu.addItem(renameItem) if tab.hasCustomTitle { - Button(String(localized: "contextMenu.removeCustomWorkspaceName", defaultValue: "Remove Custom Workspace Name")) { - tabManager.clearCustomTitle(tabId: tab.id) - } + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.removeCustomWorkspaceName", defaultValue: "Remove Custom Workspace Name") + ) { [self] in tabManager.clearCustomTitle(tabId: tab.id) }) } if !isMulti { - if let key = editWorkspaceDescriptionShortcut.keyEquivalent { - Button(String(localized: "contextMenu.editWorkspaceDescription", defaultValue: "Edit Workspace Description…")) { - beginWorkspaceDescriptionEditFromContextMenu() - } - .keyboardShortcut(key, modifiers: editWorkspaceDescriptionShortcut.eventModifiers) - } else { - Button(String(localized: "contextMenu.editWorkspaceDescription", defaultValue: "Edit Workspace Description…")) { - beginWorkspaceDescriptionEditFromContextMenu() - } - } + let editDescriptionItem = SidebarMenuItem( + title: String(localized: "contextMenu.editWorkspaceDescription", defaultValue: "Edit Workspace Description…") + ) { [self] in beginWorkspaceDescriptionEditFromContextMenu() } + applyShortcut(editWorkspaceDescriptionShortcut, to: editDescriptionItem) + menu.addItem(editDescriptionItem) if tab.hasCustomDescription { - Button(String(localized: "contextMenu.clearWorkspaceDescription", defaultValue: "Clear Workspace Description")) { - tabManager.clearCustomDescription(tabId: tab.id) - } + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.clearWorkspaceDescription", defaultValue: "Clear Workspace Description") + ) { [self] in tabManager.clearCustomDescription(tabId: tab.id) }) } - Divider() + menu.addItem(.separator()) - Button(String(localized: "contextMenu.openAgentOverview", defaultValue: "Open Agent Overview")) { - AgentOverviewWindowController.shared.show( - tabManager: tabManager, - workspaceId: tab.id - ) - } + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.openAgentOverview", defaultValue: "Open Agent Overview") + ) { [self] in + AgentOverviewWindowController.shared.show(tabManager: tabManager, workspaceId: tab.id) + }) } if !isMulti, !showsWorktreeBadge { - Divider() + menu.addItem(.separator()) if isWorktreeFolder { - Button(String(localized: "contextMenu.newWorktreeWorkspace", defaultValue: "New Worktree Workspace…")) { - promptNewWorktreeWorkspace() - } + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.newWorktreeWorkspace", defaultValue: "New Worktree Workspace…") + ) { [self] in promptNewWorktreeWorkspace() }) if worktreeChildCount > 0 { - Button( - isWorktreeFolderCollapsed - ? String(localized: "contextMenu.expandWorktreeFolder", defaultValue: "Expand Worktree Folder") - : String(localized: "contextMenu.collapseWorktreeFolder", defaultValue: "Collapse Worktree Folder") - ) { - tabManager.setWorktreeFolderCollapsed( - tab, - collapsed: !isWorktreeFolderCollapsed - ) - } + let title = isWorktreeFolderCollapsed + ? String(localized: "contextMenu.expandWorktreeFolder", defaultValue: "Expand Worktree Folder") + : String(localized: "contextMenu.collapseWorktreeFolder", defaultValue: "Collapse Worktree Folder") + menu.addItem(SidebarMenuItem(title: title) { [self] in + tabManager.setWorktreeFolderCollapsed(tab, collapsed: !isWorktreeFolderCollapsed) + }) } - Button(String(localized: "contextMenu.stopUsingAsWorktreeFolder", defaultValue: "Stop Using as Worktree Folder")) { - tabManager.disableWorktreeFolder(tab) - } + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.stopUsingAsWorktreeFolder", defaultValue: "Stop Using as Worktree Folder") + ) { [self] in tabManager.disableWorktreeFolder(tab) }) } else { - Button(String(localized: "contextMenu.useAsWorktreeFolder", defaultValue: "Use as Worktree Folder")) { - enableWorktreeFolder() - } + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.useAsWorktreeFolder", defaultValue: "Use as Worktree Folder") + ) { [self] in enableWorktreeFolder() }) } } - WorkspaceColorMenu( - hasCustomColor: tab.customColor != nil, - palette: tabColorPalette, - targetIds: targetIds, - colorScheme: colorScheme, - activeTabIndicatorStyle: activeTabIndicatorStyle, - onApplyColor: { hex, ids in applyTabColor(hex, targetIds: ids) }, - onPromptCustomColor: { ids in promptCustomColor(targetIds: ids) } + let colorItem = NSMenuItem( + title: String(localized: "contextMenu.workspaceColor", defaultValue: "Workspace Color"), + action: nil, + keyEquivalent: "" ) - .equatable() + colorItem.submenu = buildWorkspaceColorMenu(targetIds: targetIds, colorScheme: colorScheme) + menu.addItem(colorItem) - Divider() + menu.addItem(.separator()) - Button(String(localized: "contextMenu.moveUp", defaultValue: "Move Up")) { - moveBy(-1) - } - .disabled(index == 0) + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.moveUp", defaultValue: "Move Up"), + isEnabled: index != 0 + ) { [self] in moveBy(-1) }) - Button(String(localized: "contextMenu.moveDown", defaultValue: "Move Down")) { - moveBy(1) - } - .disabled(index >= tabManager.tabs.count - 1) + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.moveDown", defaultValue: "Move Down"), + isEnabled: index < tabManager.tabs.count - 1 + ) { [self] in moveBy(1) }) - Button(String(localized: "contextMenu.moveToTop", defaultValue: "Move to Top")) { + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.moveToTop", defaultValue: "Move to Top"), + isEnabled: !targetIds.isEmpty + ) { [self] in tabManager.moveTabsToTop(Set(targetIds)) syncSelectionAfterMutation() - } - .disabled(targetIds.isEmpty) + }) let referenceWindowId = AppDelegate.shared?.windowId(for: tabManager) let windowMoveTargets = AppDelegate.shared?.windowMoveTargets(referenceWindowId: referenceWindowId) ?? [] - let moveMenuTitle = targetIds.count > 1 + let moveMenuTitle = isMulti ? String(localized: "contextMenu.moveWorkspacesToWindow", defaultValue: "Move Workspaces to Window") : String(localized: "contextMenu.moveWorkspaceToWindow", defaultValue: "Move Workspace to Window") - Menu(moveMenuTitle) { - Button(String(localized: "contextMenu.newWindow", defaultValue: "New Window")) { - moveWorkspacesToNewWindow(targetIds) - } - .disabled(targetIds.isEmpty) - - if !windowMoveTargets.isEmpty { - Divider() - } - - ForEach(windowMoveTargets) { target in - Button(target.label) { - moveWorkspaces(targetIds, toWindow: target.windowId) - } - .disabled(target.isCurrentWindow || targetIds.isEmpty) - } - } - .disabled(targetIds.isEmpty) - - Divider() - - if let key = closeWorkspaceShortcut.keyEquivalent { - Button(closeLabel) { - closeTabs(targetIds, allowPinned: true) - } - .keyboardShortcut(key, modifiers: closeWorkspaceShortcut.eventModifiers) - .disabled(targetIds.isEmpty) - } else { - Button(closeLabel) { - closeTabs(targetIds, allowPinned: true) + let moveMenuItem = NSMenuItem(title: moveMenuTitle, action: nil, keyEquivalent: "") + moveMenuItem.isEnabled = !targetIds.isEmpty + let moveSubmenu = NSMenu() + moveSubmenu.autoenablesItems = false + moveSubmenu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.newWindow", defaultValue: "New Window"), + isEnabled: !targetIds.isEmpty + ) { [self] in moveWorkspacesToNewWindow(targetIds) }) + if !windowMoveTargets.isEmpty { + moveSubmenu.addItem(.separator()) + } + for target in windowMoveTargets { + moveSubmenu.addItem(SidebarMenuItem( + title: target.label, + isEnabled: !target.isCurrentWindow && !targetIds.isEmpty + ) { [self] in moveWorkspaces(targetIds, toWindow: target.windowId) }) + } + moveMenuItem.submenu = moveSubmenu + menu.addItem(moveMenuItem) + + menu.addItem(.separator()) + + let closeItem = SidebarMenuItem(title: closeLabel, isEnabled: !targetIds.isEmpty) { [self] in + closeTabs(targetIds, allowPinned: true) + } + applyShortcut(closeWorkspaceShortcut, to: closeItem) + menu.addItem(closeItem) + + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.closeOtherWorkspaces", defaultValue: "Close Other Workspaces"), + isEnabled: tabManager.tabs.count > 1 && targetIds.count != tabManager.tabs.count + ) { [self] in closeOtherTabs(targetIds) }) + + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.closeWorkspacesBelow", defaultValue: "Close Workspaces Below"), + isEnabled: index < tabManager.tabs.count - 1 + ) { [self] in closeTabsBelow(tabId: tab.id) }) + + menu.addItem(SidebarMenuItem( + title: String(localized: "contextMenu.closeWorkspacesAbove", defaultValue: "Close Workspaces Above"), + isEnabled: index != 0 + ) { [self] in closeTabsAbove(tabId: tab.id) }) + + menu.addItem(.separator()) + + menu.addItem(SidebarMenuItem( + title: markReadLabel, + isEnabled: hasUnreadNotifications(in: targetIds) + ) { [self] in markTabsRead(targetIds) }) + + menu.addItem(SidebarMenuItem( + title: markUnreadLabel, + isEnabled: hasReadNotifications(in: targetIds) + ) { [self] in markTabsUnread(targetIds) }) + + menu.addItem(SidebarMenuItem( + title: clearLatestNotificationLabel, + isEnabled: hasLatestNotifications(in: targetIds) + ) { [self] in clearLatestNotifications(targetIds) }) + + return menu + } + + private func buildWorkspaceColorMenu(targetIds: [UUID], colorScheme: ColorScheme) -> NSMenu { + let submenu = NSMenu() + submenu.autoenablesItems = false + + if tab.customColor != nil { + let clearItem = SidebarMenuItem( + title: String(localized: "contextMenu.clearColor", defaultValue: "Clear Color") + ) { [self] in applyTabColor(nil, targetIds: targetIds) } + clearItem.image = NSImage(systemSymbolName: "xmark.circle", accessibilityDescription: nil) + submenu.addItem(clearItem) + } + + let chooseCustomItem = SidebarMenuItem( + title: String(localized: "contextMenu.chooseCustomColor", defaultValue: "Choose Custom Color…") + ) { [self] in promptCustomColor(targetIds: targetIds) } + chooseCustomItem.image = NSImage(systemSymbolName: "paintpalette", accessibilityDescription: nil) + submenu.addItem(chooseCustomItem) + + let palette = WorkspaceTabColorSettings.palette() + if !palette.isEmpty { + submenu.addItem(.separator()) + } + + for entry in palette { + let swatchColor = WorkspaceTabColorSettings.displayNSColor( + hex: entry.hex, + colorScheme: colorScheme, + forceBright: activeTabIndicatorStyle == .leftRail + ) ?? NSColor(hex: entry.hex) ?? .gray + let item = SidebarMenuItem(title: entry.name) { [self] in + applyTabColor(entry.hex, targetIds: targetIds) } - .disabled(targetIds.isEmpty) - } - - Button(String(localized: "contextMenu.closeOtherWorkspaces", defaultValue: "Close Other Workspaces")) { - closeOtherTabs(targetIds) + item.image = coloredCircleImage(color: swatchColor) + submenu.addItem(item) } - .disabled(tabManager.tabs.count <= 1 || targetIds.count == tabManager.tabs.count) - Button(String(localized: "contextMenu.closeWorkspacesBelow", defaultValue: "Close Workspaces Below")) { - closeTabsBelow(tabId: tab.id) - } - .disabled(index >= tabManager.tabs.count - 1) - - Button(String(localized: "contextMenu.closeWorkspacesAbove", defaultValue: "Close Workspaces Above")) { - closeTabsAbove(tabId: tab.id) - } - .disabled(index == 0) - - Divider() - - Button(markReadLabel) { - markTabsRead(targetIds) - } - .disabled(!hasUnreadNotifications(in: targetIds)) - - Button(markUnreadLabel) { - markTabsUnread(targetIds) - } - .disabled(!hasReadNotifications(in: targetIds)) - - Button(clearLatestNotificationLabel) { - clearLatestNotifications(targetIds) - } - .disabled(!hasLatestNotifications(in: targetIds)) + return submenu } private var selectionBackgroundColor: NSColor { @@ -1180,11 +1160,16 @@ struct TabItemView: View, Equatable { if isMultiSelected { return programaAccentColor().opacity(0.25) } return Color.clear case .solidFill: - if isActive { return Color(nsColor: selectionBackgroundColor) } + // A custom color takes precedence over the plain selection wash so an + // active row keeps showing its color (the active state still reads via + // the border overlay below) instead of the custom color disappearing + // behind a flat selection fill. if let custom = resolvedCustomTabColor { + if isActive { return custom.opacity(0.85) } if isMultiSelected { return custom.opacity(0.35) } return custom.opacity(0.7) } + if isActive { return Color(nsColor: selectionBackgroundColor) } if isMultiSelected { return programaAccentColor().opacity(0.25) } return Color.clear } @@ -1195,7 +1180,12 @@ struct TabItemView: View, Equatable { } private var explicitRailColor: Color? { - guard activeTabIndicatorStyle == .leftRail || isActive, + // .leftRail always shows the custom color as an edge stripe (active or not) — + // it has no other way to carry a custom color while active. .solidFill instead + // carries the custom color directly in the row's background fill (see + // `backgroundColor` below), so it doesn't also need the rail; showing both + // would be a redundant triple indicator alongside the active border. + guard activeTabIndicatorStyle == .leftRail, let custom = resolvedCustomTabColor else { return nil } @@ -1890,6 +1880,111 @@ struct TabItemView: View, Equatable { } } +// MARK: - Native sidebar row context menu + +/// Runs `run` when its owning `NSMenuItem` is selected. Retained by the item via +/// `representedObject`, and the item is retained by its `NSMenu`, so the closure +/// lives exactly as long as the menu that owns it -- nothing to clean up between +/// context-menu presentations. +private final class SidebarMenuItemAction { + let run: () -> Void + init(_ run: @escaping () -> Void) { self.run = run } +} + +/// One statically-shared target for every closure-backed item `SidebarMenuItem` +/// builds, so building a context menu never needs its own retained NSObject target. +private final class SidebarMenuItemTarget: NSObject { + static let shared = SidebarMenuItemTarget() + + @objc func invoke(_ sender: NSMenuItem) { + (sender.representedObject as? SidebarMenuItemAction)?.run() + } +} + +/// Builds a closure-backed `NSMenuItem`, the AppKit equivalent of a SwiftUI +/// `Button { handler() } label: { Text(title) }.disabled(!isEnabled)` inside a +/// `.contextMenu`. +private func SidebarMenuItem( + title: String, + isEnabled: Bool = true, + handler: @escaping () -> Void +) -> NSMenuItem { + let item = NSMenuItem(title: title, action: #selector(SidebarMenuItemTarget.invoke(_:)), keyEquivalent: "") + item.target = SidebarMenuItemTarget.shared + item.representedObject = SidebarMenuItemAction(handler) + item.isEnabled = isEnabled + return item +} + +/// Native right-click/ctrl-click catcher for the sidebar row's context menu, built +/// fresh at click time instead of hanging a SwiftUI `.contextMenu` off the row's +/// body (see the `.overlay` call site in `TabItemView.body` for why). Mirrors +/// `MiddleClickCapture` (Sources/SidebarVisuals.swift): `hitTest` only claims the +/// view for the button/modifier combo it cares about, so left-click selection, +/// hover, and drag continue to hit-test through to SwiftUI/AppKit normally. +private struct SidebarRowContextMenuCapture: NSViewRepresentable { + let buildMenu: () -> NSMenu + + func makeNSView(context: Context) -> SidebarRowContextMenuCaptureView { + let view = SidebarRowContextMenuCaptureView() + view.buildMenu = buildMenu + return view + } + + func updateNSView(_ nsView: SidebarRowContextMenuCaptureView, context: Context) { + nsView.buildMenu = buildMenu + } +} + +private final class SidebarRowContextMenuCaptureView: NSView { + var buildMenu: (() -> NSMenu)? + + override func hitTest(_ point: NSPoint) -> NSView? { + // Intercept right-mouse-down, and control-click too: unlike a plain + // right-click, control-click arrives here as a genuine .leftMouseDown + // event with the .control modifier set -- AppKit only redirects it to + // rightMouseDown(with:) dispatch for whichever view hit-testing already + // picked, so hitTest itself must recognize the control-click case or + // this view is never chosen and the click falls through to plain + // left-click selection underneath. Every other click (plain left-click + // selection, hover, drag-reorder) continues to hit-test through to + // SwiftUI/AppKit normally. + guard let event = NSApp.currentEvent else { return nil } + if event.type == .rightMouseDown { + return self + } + if event.type == .leftMouseDown, event.modifierFlags.contains(.control) { + return self + } + return nil + } + + override func mouseDown(with event: NSEvent) { + guard event.modifierFlags.contains(.control) else { + super.mouseDown(with: event) + return + } + rightMouseDown(with: event) + } + + override func rightMouseDown(with event: NSEvent) { + guard let menu = buildMenu?() else { + super.rightMouseDown(with: event) + return + } + NSMenu.popUpContextMenu(menu, with: event, for: self) + } + + // VoiceOver's "Show menu" action relied on SwiftUI's `.contextMenu` before this + // change; best-effort parity for a plain NSView. Not exercised by this change + // (no VoiceOver pass was run) -- report a regression here if one surfaces. + override func accessibilityPerformShowMenu() -> Bool { + guard let menu = buildMenu?() else { return false } + menu.popUp(positioning: nil, at: NSPoint(x: bounds.midX, y: bounds.midY), in: self) + return true + } +} + private struct SidebarWorkspaceDescriptionText: View { let markdown: String let isActive: Bool From 07c2a487176a76a688e2a91bfb9aed1e1b7e8842 Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:08:44 -0300 Subject: [PATCH 11/19] chore: scope workflow tokens to read and document the zig skip --- .github/workflows/build-ghosttykit.yml | 3 +++ .github/workflows/ci-macos-compat.yml | 3 +++ .github/workflows/ci.yml | 3 +++ .github/workflows/test-depot.yml | 3 +++ .github/workflows/test-e2e.yml | 3 +++ CONTRIBUTING.md | 9 +++++++++ 6 files changed, 24 insertions(+) diff --git a/.github/workflows/build-ghosttykit.yml b/.github/workflows/build-ghosttykit.yml index 682a26b52..c4c38601a 100644 --- a/.github/workflows/build-ghosttykit.yml +++ b/.github/workflows/build-ghosttykit.yml @@ -32,6 +32,9 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +permissions: + contents: read + jobs: build-ghosttykit: runs-on: macos-15 diff --git a/.github/workflows/ci-macos-compat.yml b/.github/workflows/ci-macos-compat.yml index ec6d55343..6f2b17e5a 100644 --- a/.github/workflows/ci-macos-compat.yml +++ b/.github/workflows/ci-macos-compat.yml @@ -21,6 +21,9 @@ on: workflow_dispatch: +permissions: + contents: read + jobs: change-detection: runs-on: ubuntu-latest diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3792c468b..0bfa77bcd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,6 +68,9 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +permissions: + contents: read + jobs: change-detection: runs-on: ubuntu-latest diff --git a/.github/workflows/test-depot.yml b/.github/workflows/test-depot.yml index dfba74b84..6ad413933 100644 --- a/.github/workflows/test-depot.yml +++ b/.github/workflows/test-depot.yml @@ -26,6 +26,9 @@ on: required: false default: "120" +permissions: + contents: read + jobs: tests: runs-on: macos-15 diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index 9933e6471..ff4442a0b 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -29,6 +29,9 @@ on: - macos-15 - macos-14 +permissions: + contents: read + jobs: e2e: runs-on: ${{ inputs.runner || 'macos-15' }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e8f846ea3..968dc99c7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,6 +38,15 @@ in [docs/windows-testing.md](docs/windows-testing.md). ``` The script prints the `.app` path. Cmd-click to open, or pass `--launch` to open automatically. + If your local Zig doesn't match the version the ghostty submodule needs + (`scripts/required-zig-version.sh`), skip the Zig steps and build against the + existing GhosttyKit.xcframework: + ```bash + PROGRAMA_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag my-feature + ``` + If the xcframework is missing, `./scripts/download-prebuilt-ghosttykit.sh` + fetches a prebuilt copy. + ## Development Scripts | Script | Description | From e83b19b5d35955575260deb208d16b47ed7cf5cc Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:11:16 -0300 Subject: [PATCH 12/19] refactor: tighten sidebar menu hit-testing and comments --- Sources/TabItemView.swift | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/Sources/TabItemView.swift b/Sources/TabItemView.swift index c166a61ac..bf571c194 100644 --- a/Sources/TabItemView.swift +++ b/Sources/TabItemView.swift @@ -746,15 +746,11 @@ struct TabItemView: View, Equatable { } } .overlay { - // Native AppKit context menu, built fresh from live state at click time, - // instead of SwiftUI's `.contextMenu`. `.contextMenu`'s content closure - // hangs off this row's body, which re-evaluates as often as every 40ms - // from the onReceive handlers above (`workspaceObservationGeneration`), - // on hover changes, and on any ==-compared param change. SwiftUI rebuilds - // an already-open `.contextMenu` on every one of those re-evaluations, - // and macOS reads that rebuild as the menu (and any open submenu, e.g. - // Workspace Color) repeatedly closing and reopening. Building the NSMenu - // only once, at the moment of the click, sidesteps the churn entirely. + // Native AppKit context menu, built from live state at click time. Do not + // use SwiftUI's `.contextMenu` here: this row's body re-evaluates as often + // as every 40ms (`workspaceObservationGeneration`, hover, param changes), + // SwiftUI rebuilds an open `.contextMenu` on each re-evaluation, and macOS + // closes the menu and any open submenu (e.g. Workspace Color) when it does. SidebarRowContextMenuCapture { #if DEBUG dlog("sidebar.contextMenu.open workspace=\(tab.id.uuidString.prefix(5)) targets=\(contextMenuWorkspaceIds.count)") @@ -887,22 +883,15 @@ struct TabItemView: View, Equatable { isMulti ? multi : single } - /// Applies a `StoredShortcut`'s key equivalent to an `NSMenuItem`, mirroring the - /// `if let key = shortcut.keyEquivalent { .keyboardShortcut(key, modifiers:) }` - /// pattern the SwiftUI menu used to use. `menuItemKeyEquivalent`/`modifierFlags` - /// are the AppKit-side counterparts of `keyEquivalent`/`eventModifiers` and are - /// `nil`/empty under the same conditions (chorded shortcuts have no single - /// menu-item key equivalent). + /// Shows a `StoredShortcut` on an `NSMenuItem`. Chorded shortcuts have no single + /// menu-item key equivalent, so they show none. private func applyShortcut(_ shortcut: StoredShortcut, to item: NSMenuItem) { guard let key = shortcut.menuItemKeyEquivalent else { return } item.keyEquivalent = key item.keyEquivalentModifierMask = shortcut.modifierFlags } - /// Builds the sidebar row's context menu fresh from live state at click time, - /// replacing the SwiftUI `.contextMenu` this row used to carry. Every item here - /// ports the SwiftUI menu 1:1 (same strings/keys, order, dividers, conditional - /// visibility, disabled states, and shortcut display). + /// Builds the sidebar row's context menu from live state at click time. private func buildWorkspaceContextMenu(colorScheme: ColorScheme) -> NSMenu { let targetIds = contextMenuWorkspaceIds let isMulti = targetIds.count > 1 @@ -1949,7 +1938,9 @@ private final class SidebarRowContextMenuCaptureView: NSView { // left-click selection underneath. Every other click (plain left-click // selection, hover, drag-reorder) continues to hit-test through to // SwiftUI/AppKit normally. - guard let event = NSApp.currentEvent else { return nil } + guard let event = NSApp.currentEvent, + let superview, + bounds.contains(convert(point, from: superview)) else { return nil } if event.type == .rightMouseDown { return self } @@ -1975,9 +1966,7 @@ private final class SidebarRowContextMenuCaptureView: NSView { NSMenu.popUpContextMenu(menu, with: event, for: self) } - // VoiceOver's "Show menu" action relied on SwiftUI's `.contextMenu` before this - // change; best-effort parity for a plain NSView. Not exercised by this change - // (no VoiceOver pass was run) -- report a regression here if one surfaces. + // VoiceOver "Show menu" support for this view. override func accessibilityPerformShowMenu() -> Bool { guard let menu = buildMenu?() else { return false } menu.popUp(positioning: nil, at: NSPoint(x: bounds.midX, y: bounds.midY), in: self) From 6b7dbfac1dd28898eecc77fb1cbe6720681976b3 Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:11:16 -0300 Subject: [PATCH 13/19] feat: color-code pane tabs with a tinted background --- Resources/Localizable.xcstrings | 17 + Sources/SessionPersistence.swift | 1 + Sources/TabManager.swift | 11 + Sources/Workspace+Bonsplit.swift | 11 + Sources/Workspace+Persistence.swift | 3 + Sources/Workspace+SidebarTelemetry.swift | 3 + Sources/Workspace.swift | 80 +++- programaTests/WorkspaceUnitTests.swift | 43 ++ .../Bonsplit/Internal/Models/TabItem.swift | 8 +- .../Bonsplit/Internal/Views/TabBarView.swift | 9 +- .../Bonsplit/Internal/Views/TabItemView.swift | 419 +++++++++++++----- .../Bonsplit/Public/BonsplitController.swift | 25 +- .../Bonsplit/Public/BonsplitDelegate.swift | 4 + .../Sources/Bonsplit/Public/Types/Tab.swift | 7 +- .../Public/Types/TabContextAction.swift | 16 + .../Resources/en.lproj/Localizable.strings | 3 + .../Resources/ja.lproj/Localizable.strings | 3 + 17 files changed, 552 insertions(+), 111 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 1d615249d..9c7160076 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -2842,6 +2842,23 @@ } } }, + "alert.customTabColor.title": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Custom Tab Color" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "タブのカスタムカラー" + } + } + } + }, "alert.invalidColor.emptyMessage": { "extractionState": "manual", "localizations": { diff --git a/Sources/SessionPersistence.swift b/Sources/SessionPersistence.swift index 051f0462c..6044731da 100644 --- a/Sources/SessionPersistence.swift +++ b/Sources/SessionPersistence.swift @@ -292,6 +292,7 @@ struct SessionPanelSnapshot: Codable, Sendable { var customTitle: String? var directory: String? var isPinned: Bool + var customColorHex: String? var isManuallyUnread: Bool var gitBranch: SessionGitBranchSnapshot? var listeningPorts: [Int] diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 15563587d..d2c9efd17 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -336,6 +336,17 @@ enum WorkspaceTabColorSettings { return Color(nsColor: color) } + /// Palette entries for bonsplit's native tab-color context menu, resolved for the app's + /// current effective appearance. Bonsplit cannot import app types, so this maps the + /// shared palette into its own `TabColorSwatch` value type at call time. + static func tabColorSwatches(defaults: UserDefaults = .standard) -> [TabColorSwatch] { + let colorScheme: ColorScheme = NSApp.effectiveAppearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua ? .dark : .light + return palette(defaults: defaults).compactMap { entry in + guard let color = displayNSColor(hex: entry.hex, colorScheme: colorScheme) else { return nil } + return TabColorSwatch(name: entry.name, hex: entry.hex, swatchColor: color) + } + } + static func displayNSColor( hex: String, colorScheme: ColorScheme, diff --git a/Sources/Workspace+Bonsplit.swift b/Sources/Workspace+Bonsplit.swift index 5d84ceec4..3ea48ef33 100644 --- a/Sources/Workspace+Bonsplit.swift +++ b/Sources/Workspace+Bonsplit.swift @@ -596,6 +596,7 @@ extension Workspace: @preconcurrency BonsplitDelegate { kind: surfaceKind(for: panel), isLoading: browserPanel?.isLoading ?? false, isPinned: pinnedPanelIds.contains(panelId), + customColorHex: panelColorHexes[panelId], directory: panelDirectories[panelId], ttyName: surfaceTTYNames[panelId], cachedTitle: cachedTitle, @@ -1078,11 +1079,21 @@ extension Workspace: @preconcurrency BonsplitDelegate { case .toggleZoom: guard let panelId = panelIdFromSurfaceId(tab.id) else { return } toggleSplitZoom(panelId: panelId) + case .clearTabColor: + guard let panelId = panelIdFromSurfaceId(tab.id) else { return } + setPanelColor(panelId: panelId, hex: nil) + case .chooseCustomTabColor: + promptCustomTabColor(tabId: tab.id) @unknown default: break } } + func splitTabBar(_ controller: BonsplitController, didRequestTabColor hex: String, for tab: Bonsplit.Tab, inPane pane: PaneID) { + guard let panelId = panelIdFromSurfaceId(tab.id) else { return } + setPanelColor(panelId: panelId, hex: hex) + } + func splitTabBar(_ controller: BonsplitController, didChangeGeometry snapshot: LayoutSnapshot) { tmuxLayoutSnapshot = snapshot scheduleTerminalGeometryReconcile() diff --git a/Sources/Workspace+Persistence.swift b/Sources/Workspace+Persistence.swift index cc17fa9a8..13124b86b 100644 --- a/Sources/Workspace+Persistence.swift +++ b/Sources/Workspace+Persistence.swift @@ -282,6 +282,7 @@ extension Workspace { let customTitle = panelCustomTitles[panelId] let directory = panelDirectories[panelId] let isPinned = pinnedPanelIds.contains(panelId) + let customColorHex = panelColorHexes[panelId] let isManuallyUnread = manualUnreadPanelIds.contains(panelId) let branchSnapshot = panelGitBranches[panelId].map { SessionGitBranchSnapshot(branch: $0.branch, isDirty: $0.isDirty) @@ -359,6 +360,7 @@ extension Workspace { customTitle: customTitle, directory: directory, isPinned: isPinned, + customColorHex: customColorHex, isManuallyUnread: isManuallyUnread, gitBranch: branchSnapshot, listeningPorts: listeningPorts, @@ -766,6 +768,7 @@ extension Workspace { setPanelCustomTitle(panelId: panelId, title: snapshot.customTitle) setPanelPinned(panelId: panelId, pinned: snapshot.isPinned) + setPanelColor(panelId: panelId, hex: snapshot.customColorHex) if snapshot.isManuallyUnread { markPanelUnread(panelId) diff --git a/Sources/Workspace+SidebarTelemetry.swift b/Sources/Workspace+SidebarTelemetry.swift index 01eaa5fa8..2984b8474 100644 --- a/Sources/Workspace+SidebarTelemetry.swift +++ b/Sources/Workspace+SidebarTelemetry.swift @@ -447,6 +447,9 @@ extension Workspace { if pinnedPanelIds.contains(where: { !validSurfaceIds.contains($0) }) { pinnedPanelIds = pinnedPanelIds.filter { validSurfaceIds.contains($0) } } + if panelColorHexes.keys.contains(where: { !validSurfaceIds.contains($0) }) { + panelColorHexes = panelColorHexes.filter { validSurfaceIds.contains($0.key) } + } if manualUnreadPanelIds.contains(where: { !validSurfaceIds.contains($0) }) { manualUnreadPanelIds = manualUnreadPanelIds.filter { validSurfaceIds.contains($0) } } diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index a43dd332b..f152171d0 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -142,6 +142,9 @@ final class Workspace: Identifiable, ObservableObject { var panelsWithLiveTitle: Set = [] @Published var panelCustomTitles: [UUID: String] = [:] @Published var pinnedPanelIds: Set = [] + /// Per-tab color tint (pane tab bar), keyed by panel id. Distinct from `customColor`, + /// which is the sidebar's per-workspace color. + @Published var panelColorHexes: [UUID: String] = [:] @Published var manualUnreadPanelIds: Set = [] @Published var tmuxLayoutSnapshot: LayoutSnapshot? @Published private(set) var tmuxWorkspaceFlashPanelId: UUID? @@ -400,6 +403,7 @@ final class Workspace: Identifiable, ObservableObject { // Set ourselves as delegate bonsplitController.delegate = self + bonsplitController.tabColorPaletteProvider = { WorkspaceTabColorSettings.tabColorSwatches() } connectSharedTabOrdering() // Ensure bonsplit has a focused pane and our didSelectTab handler runs for the @@ -497,6 +501,7 @@ final class Workspace: Identifiable, ObservableObject { } bonsplitController.delegate = self + bonsplitController.tabColorPaletteProvider = { WorkspaceTabColorSettings.tabColorSwatches() } connectSharedTabOrdering() if let initialTabId { @@ -612,6 +617,7 @@ final class Workspace: Identifiable, ObservableObject { let kind: String? let isLoading: Bool let isPinned: Bool + let customColorHex: String? let directory: String? let ttyName: String? let cachedTitle: String? @@ -628,6 +634,7 @@ final class Workspace: Identifiable, ObservableObject { kind: String?, isLoading: Bool, isPinned: Bool, + customColorHex: String?, directory: String?, ttyName: String?, cachedTitle: String?, @@ -642,6 +649,7 @@ final class Workspace: Identifiable, ObservableObject { self.kind = kind self.isLoading = isLoading self.isPinned = isPinned + self.customColorHex = customColorHex self.directory = directory self.ttyName = ttyName self.cachedTitle = cachedTitle @@ -976,14 +984,16 @@ final class Workspace: Identifiable, ObservableObject { func syncPinnedStateForTab(_ tabId: TabID, panelId: UUID) { let isPinned = pinnedPanelIds.contains(panelId) + let colorHex = panelColorHexes[panelId] if let panel = panels[panelId] { bonsplitController.updateTab( tabId, kind: .some(surfaceKind(for: panel)), - isPinned: isPinned + isPinned: isPinned, + customColorHex: .some(colorHex) ) } else { - bonsplitController.updateTab(tabId, isPinned: isPinned) + bonsplitController.updateTab(tabId, isPinned: isPinned, customColorHex: .some(colorHex)) } } @@ -1152,6 +1162,20 @@ final class Workspace: Identifiable, ObservableObject { normalizePinnedTabs(in: paneId) } + func setPanelColor(panelId: UUID, hex: String?) { + guard panels[panelId] != nil else { return } + let normalized = hex.flatMap { WorkspaceTabColorSettings.normalizedHex($0) } + guard panelColorHexes[panelId] != normalized else { return } + if let normalized { + panelColorHexes[panelId] = normalized + } else { + panelColorHexes.removeValue(forKey: panelId) + } + + guard let tabId = surfaceIdFromPanelId(panelId) else { return } + bonsplitController.updateTab(tabId, customColorHex: .some(normalized)) + } + func markPanelUnread(_ panelId: UUID) { guard panels[panelId] != nil else { return } guard manualUnreadPanelIds.insert(panelId).inserted else { return } @@ -1778,6 +1802,11 @@ final class Workspace: Identifiable, ObservableObject { } else { pinnedPanelIds.remove(detached.panelId) } + if let customColorHex = detached.customColorHex { + panelColorHexes[detached.panelId] = customColorHex + } else { + panelColorHexes.removeValue(forKey: detached.panelId) + } if detached.manuallyUnread { manualUnreadPanelIds.insert(detached.panelId) manualUnreadMarkedAt[detached.panelId] = .distantPast @@ -1795,6 +1824,7 @@ final class Workspace: Identifiable, ObservableObject { isDirty: detached.panel.isDirty, isLoading: detached.isLoading, isPinned: detached.isPinned, + customColorHex: detached.customColorHex, inPane: paneId ) else { panels.removeValue(forKey: detached.panelId) @@ -1803,6 +1833,7 @@ final class Workspace: Identifiable, ObservableObject { panelTitles.removeValue(forKey: detached.panelId) panelCustomTitles.removeValue(forKey: detached.panelId) pinnedPanelIds.remove(detached.panelId) + panelColorHexes.removeValue(forKey: detached.panelId) manualUnreadPanelIds.remove(detached.panelId) manualUnreadMarkedAt.removeValue(forKey: detached.panelId) panelSubscriptions.removeValue(forKey: detached.panelId) @@ -2123,6 +2154,51 @@ final class Workspace: Identifiable, ObservableObject { setPanelCustomTitle(panelId: panelId, title: input.stringValue) } + func promptCustomTabColor(tabId: TabID) { + guard let panelId = panelIdFromSurfaceId(tabId) else { return } + + let alert = NSAlert() + alert.messageText = String(localized: "alert.customTabColor.title", defaultValue: "Custom Tab Color") + alert.informativeText = String(localized: "alert.customColor.message", defaultValue: "Enter a hex color in the format #RRGGBB.") + + let seed = panelColorHexes[panelId] ?? WorkspaceTabColorSettings.customPaletteEntries().first?.hex ?? "" + let input = NSTextField(string: seed) + input.placeholderString = "#1565C0" + input.frame = NSRect(x: 0, y: 0, width: 240, height: 22) + alert.accessoryView = input + alert.addButton(withTitle: String(localized: "alert.customColor.apply", defaultValue: "Apply")) + alert.addButton(withTitle: String(localized: "alert.customColor.cancel", defaultValue: "Cancel")) + + let alertWindow = alert.window + alertWindow.initialFirstResponder = input + DispatchQueue.main.async { + alertWindow.makeFirstResponder(input) + input.selectText(nil) + } + + let response = alert.runModal() + guard response == .alertFirstButtonReturn else { return } + guard let normalized = WorkspaceTabColorSettings.addCustomColor(input.stringValue) else { + showInvalidTabColorAlert(input.stringValue) + return + } + setPanelColor(panelId: panelId, hex: normalized) + } + + private func showInvalidTabColorAlert(_ value: String) { + let alert = NSAlert() + alert.alertStyle = .warning + alert.messageText = String(localized: "alert.invalidColor.title", defaultValue: "Invalid Color") + let trimmed = value.trimmingCharacters(in: .whitespacesAndNewlines) + if trimmed.isEmpty { + alert.informativeText = String(localized: "alert.invalidColor.emptyMessage", defaultValue: "Enter a hex color in the format #RRGGBB.") + } else { + alert.informativeText = String(localized: "alert.invalidColor.invalidMessage", defaultValue: "\"\(trimmed)\" is not a valid hex color. Use #RRGGBB.") + } + alert.addButton(withTitle: String(localized: "alert.invalidColor.ok", defaultValue: "OK")) + _ = alert.runModal() + } + enum PanelMoveDestination { case newWorkspaceInCurrentWindow case selectedWorkspaceInNewWindow diff --git a/programaTests/WorkspaceUnitTests.swift b/programaTests/WorkspaceUnitTests.swift index 39bc630f0..a48ac4865 100644 --- a/programaTests/WorkspaceUnitTests.swift +++ b/programaTests/WorkspaceUnitTests.swift @@ -5342,3 +5342,46 @@ final class FocusTransitionCoordinatorTests: XCTestCase { ) } } + +final class SessionPanelSnapshotTabColorTests: XCTestCase { + private func makeSnapshot(customColorHex: String?) -> SessionPanelSnapshot { + SessionPanelSnapshot( + id: UUID(), + type: .terminal, + title: "Terminal", + customTitle: nil, + directory: nil, + isPinned: false, + customColorHex: customColorHex, + isManuallyUnread: false, + gitBranch: nil, + listeningPorts: [], + ttyName: nil, + terminal: SessionTerminalPanelSnapshot(workingDirectory: nil, scrollback: nil), + browser: nil, + markdown: nil, + review: nil + ) + } + + func testCustomColorHexRoundTripsThroughEncodeDecode() throws { + let original = makeSnapshot(customColorHex: "#1565C0") + let data = try JSONEncoder().encode(original) + let decoded = try JSONDecoder().decode(SessionPanelSnapshot.self, from: data) + XCTAssertEqual(decoded.customColorHex, "#1565C0") + XCTAssertEqual(decoded.id, original.id) + } + + func testMissingCustomColorHexKeyDecodesToNil() throws { + // Simulates a pre-tab-color session snapshot written before this field existed. + let legacySnapshot = makeSnapshot(customColorHex: nil) + var data = try JSONEncoder().encode(legacySnapshot) + var object = try JSONSerialization.jsonObject(with: data) as! [String: Any] + XCTAssertNil(object["customColorHex"], "legacy fixture must not already contain the key") + object.removeValue(forKey: "customColorHex") + data = try JSONSerialization.data(withJSONObject: object) + + let decoded = try JSONDecoder().decode(SessionPanelSnapshot.self, from: data) + XCTAssertNil(decoded.customColorHex) + } +} diff --git a/vendor/bonsplit/Sources/Bonsplit/Internal/Models/TabItem.swift b/vendor/bonsplit/Sources/Bonsplit/Internal/Models/TabItem.swift index 4436760f0..df5603b3a 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Internal/Models/TabItem.swift +++ b/vendor/bonsplit/Sources/Bonsplit/Internal/Models/TabItem.swift @@ -25,6 +25,7 @@ struct TabItem: Identifiable, Hashable, Codable { var showsNotificationBadge: Bool var isLoading: Bool var isPinned: Bool + var customColorHex: String? init( id: UUID = UUID(), @@ -36,7 +37,8 @@ struct TabItem: Identifiable, Hashable, Codable { isDirty: Bool = false, showsNotificationBadge: Bool = false, isLoading: Bool = false, - isPinned: Bool = false + isPinned: Bool = false, + customColorHex: String? = nil ) { self.id = id self.title = title @@ -48,6 +50,7 @@ struct TabItem: Identifiable, Hashable, Codable { self.showsNotificationBadge = showsNotificationBadge self.isLoading = isLoading self.isPinned = isPinned + self.customColorHex = customColorHex } func hash(into hasher: inout Hasher) { @@ -69,6 +72,7 @@ struct TabItem: Identifiable, Hashable, Codable { case showsNotificationBadge case isLoading case isPinned + case customColorHex } init(from decoder: Decoder) throws { @@ -83,6 +87,7 @@ struct TabItem: Identifiable, Hashable, Codable { self.showsNotificationBadge = try c.decodeIfPresent(Bool.self, forKey: .showsNotificationBadge) ?? false self.isLoading = try c.decodeIfPresent(Bool.self, forKey: .isLoading) ?? false self.isPinned = try c.decodeIfPresent(Bool.self, forKey: .isPinned) ?? false + self.customColorHex = try c.decodeIfPresent(String.self, forKey: .customColorHex) } func encode(to encoder: Encoder) throws { @@ -97,6 +102,7 @@ struct TabItem: Identifiable, Hashable, Codable { try c.encode(showsNotificationBadge, forKey: .showsNotificationBadge) try c.encode(isLoading, forKey: .isLoading) try c.encode(isPinned, forKey: .isPinned) + try c.encodeIfPresent(customColorHex, forKey: .customColorHex) } } diff --git a/vendor/bonsplit/Sources/Bonsplit/Internal/Views/TabBarView.swift b/vendor/bonsplit/Sources/Bonsplit/Internal/Views/TabBarView.swift index aa7768feb..3c596b76d 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Internal/Views/TabBarView.swift +++ b/vendor/bonsplit/Sources/Bonsplit/Internal/Views/TabBarView.swift @@ -46,6 +46,8 @@ struct TabContextMenuState { let isZoomed: Bool let hasSplits: Bool let shortcuts: [TabContextAction: KeyboardShortcut] + let hasCustomColor: Bool + let colorPalette: [TabColorSwatch] var canMarkAsUnread: Bool { !isUnread @@ -310,6 +312,9 @@ struct TabBarView: View { }, onContextAction: { action in controller.requestTabContextAction(action, for: TabID(id: tab.id), inPane: pane.id) + }, + onApplyTabColor: { hex in + controller.requestTabColor(hex, for: TabID(id: tab.id), inPane: pane.id) } ) .background( @@ -368,7 +373,9 @@ struct TabBarView: View { canMoveToRightPane: controller.adjacentPane(to: pane.id, direction: .right) != nil, isZoomed: splitViewController.zoomedPaneId == pane.id, hasSplits: splitViewController.rootNode.allPaneIds.count > 1, - shortcuts: controller.contextMenuShortcuts + shortcuts: controller.contextMenuShortcuts, + hasCustomColor: tab.customColorHex != nil, + colorPalette: controller.tabColorPaletteProvider?() ?? [] ) } diff --git a/vendor/bonsplit/Sources/Bonsplit/Internal/Views/TabItemView.swift b/vendor/bonsplit/Sources/Bonsplit/Internal/Views/TabItemView.swift index 1b18d233d..4f56cfa16 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Internal/Views/TabItemView.swift +++ b/vendor/bonsplit/Sources/Bonsplit/Internal/Views/TabItemView.swift @@ -33,6 +33,18 @@ enum TabItemStyling { } return existing } + + /// Parses a "#RRGGBB" hex string into a Color. Bonsplit is host-app-agnostic, so it + /// cannot reuse the app's own NSColor(hex:) extension. + static func color(fromHex hex: String) -> Color? { + var body = hex.trimmingCharacters(in: .whitespacesAndNewlines) + if body.hasPrefix("#") { body.removeFirst() } + guard body.count == 6, let value = UInt64(body, radix: 16) else { return nil } + let r = Double((value >> 16) & 0xFF) / 255.0 + let g = Double((value >> 8) & 0xFF) / 255.0 + let b = Double(value & 0xFF) / 255.0 + return Color(red: r, green: g, blue: b) + } } /// Individual tab view with icon, title, close button, and dirty indicator @@ -50,6 +62,7 @@ struct TabItemView: View { let onClose: () -> Void let onZoomToggle: () -> Void let onContextAction: (TabContextAction) -> Void + let onApplyTabColor: (String) -> Void @State private var isHovered = false @State private var isCloseHovered = false @@ -197,9 +210,17 @@ struct TabItemView: View { // Keep icon rendering stable while hovering; only accessory/background elements animate. isHovered = hovering } - .contextMenu { - contextMenuContent - } + // Right-click / ctrl-click context menu, built natively with NSMenu at click time. + // A SwiftUI `.contextMenu` here gets closed by macOS the instant this view re-renders + // while the menu is open (constant re-eval from closure props, hover state changes when + // the pointer crosses onto the open menu, etc.) -- AppKit's own tracking loop is immune + // to that since it owns the run loop for the popup, not SwiftUI's diffing. + .background(TabContextMenuHostView( + tab: tab, + contextMenuState: contextMenuState, + onContextAction: onContextAction, + onApplyTabColor: onApplyTabColor + )) .accessibilityElement(children: .combine) .accessibilityLabel(tab.title) .accessibilityValue(accessibilityValue) @@ -349,111 +370,17 @@ struct TabItemView: View { return parts.joined(separator: ", ") } - @ViewBuilder - private var contextMenuContent: some View { - contextButton("Rename Tab…", action: .rename) - - if contextMenuState.hasCustomTitle { - contextButton("Remove Custom Tab Name", action: .clearName) - } - - Divider() - - contextButton("Close Tabs to Left", action: .closeToLeft) - .disabled(!contextMenuState.canCloseToLeft) - - contextButton("Close Tabs to Right", action: .closeToRight) - .disabled(!contextMenuState.canCloseToRight) - - contextButton("Close Other Tabs", action: .closeOthers) - .disabled(!contextMenuState.canCloseOthers) - - contextButton("Move Tab…", action: .move) - - if contextMenuState.isTerminal { - localizedContextButton( - "command.moveTabToLeftPane.title", - defaultValue: "Move to Left Pane", - action: .moveToLeftPane - ) - .disabled(!contextMenuState.canMoveToLeftPane) - - localizedContextButton( - "command.moveTabToRightPane.title", - defaultValue: "Move to Right Pane", - action: .moveToRightPane - ) - .disabled(!contextMenuState.canMoveToRightPane) - } - - Divider() - - contextButton("New Terminal Tab to Right", action: .newTerminalToRight) - - contextButton("New Browser Tab to Right", action: .newBrowserToRight) - - if contextMenuState.isBrowser { - Divider() - - contextButton("Reload Tab", action: .reload) - - contextButton("Duplicate Tab", action: .duplicate) - } - - Divider() - - if contextMenuState.hasSplits { - contextButton( - contextMenuState.isZoomed ? "Exit Zoom" : "Zoom Pane", - action: .toggleZoom - ) - } - - contextButton( - contextMenuState.isPinned ? "Unpin Tab" : "Pin Tab", - action: .togglePin - ) - - if contextMenuState.isUnread { - contextButton("Mark Tab as Read", action: .markAsRead) - .disabled(!contextMenuState.canMarkAsRead) - } else { - contextButton("Mark Tab as Unread", action: .markAsUnread) - .disabled(!contextMenuState.canMarkAsUnread) - } - } - - @ViewBuilder - private func contextButton(_ title: String, action: TabContextAction) -> some View { - if let shortcut = contextMenuState.shortcuts[action] { - Button(title) { - onContextAction(action) - } - .keyboardShortcut(shortcut) - } else { - Button(title) { - onContextAction(action) - } - } - } - - @ViewBuilder - private func localizedContextButton( - _ titleKey: String, - defaultValue: String, - action: TabContextAction - ) -> some View { - contextButton( - Bundle.module.localizedString(forKey: titleKey, value: defaultValue, table: nil), - action: action - ) - } - // MARK: - Tab Background @ViewBuilder private var tabBackground: some View { ZStack(alignment: .top) { + // Custom tab color tint (bottom-most layer, low-opacity wash across the whole tab). + if let hex = tab.customColorHex, let tintColor = TabItemStyling.color(fromHex: hex) { + Rectangle() + .fill(tintColor.opacity(0.2)) + } + // Background fill (hover) if TabItemStyling.shouldShowHoverBackground(isHovered: isHovered, isSelected: isSelected) { Rectangle() @@ -663,3 +590,289 @@ private struct MiddleClickMonitorView: NSViewRepresentable { context.coordinator.onMiddleClick = onMiddleClick } } + +/// Retains a single closure as an NSMenuItem target. `NSMenuItem.target` is weak, so each +/// item's target must be kept alive independently until the menu is dismissed. +private final class ClosureMenuItemTarget: NSObject { + let handler: () -> Void + + init(_ handler: @escaping () -> Void) { + self.handler = handler + } + + @objc func invoke() { + handler() + } +} + +/// Right-click / ctrl-click hook that builds and pops a native NSMenu at click time from the +/// tab's current context-menu state. AppKit owns the menu's tracking run loop, so SwiftUI +/// re-renders of the tab row (which happen constantly -- see the comment at the `.contextMenu` +/// replacement call site) can never close it out from under the user, unlike SwiftUI's own +/// `.contextMenu` modifier. +private struct TabContextMenuHostView: NSViewRepresentable { + let tab: TabItem + let contextMenuState: TabContextMenuState + let onContextAction: (TabContextAction) -> Void + let onApplyTabColor: (String) -> Void + + final class Coordinator { + var tab: TabItem? + var contextMenuState: TabContextMenuState? + var onContextAction: ((TabContextAction) -> Void)? + var onApplyTabColor: ((String) -> Void)? + weak var view: NSView? + var monitor: Any? + // Keeps this popup's NSMenuItem targets alive for the duration of the menu; replaced + // (and the previous batch released) on every new popup. + var retainedTargets: [ClosureMenuItemTarget] = [] + + deinit { + if let monitor { + NSEvent.removeMonitor(monitor) + } + } + + func popUpMenu(for event: NSEvent, in view: NSView) { + guard let tab, let contextMenuState else { return } + var targets: [ClosureMenuItemTarget] = [] + let menu = Self.buildMenu( + tab: tab, + state: contextMenuState, + onContextAction: { [weak self] action in self?.onContextAction?(action) }, + onApplyTabColor: { [weak self] hex in self?.onApplyTabColor?(hex) }, + retaining: &targets + ) + retainedTargets = targets + NSMenu.popUpContextMenu(menu, with: event, for: view) + } + + private static func addItem( + to menu: NSMenu, + title: String, + action: TabContextAction, + enabled: Bool, + shortcut: KeyboardShortcut?, + onContextAction: @escaping (TabContextAction) -> Void, + retaining targets: inout [ClosureMenuItemTarget] + ) { + let target = ClosureMenuItemTarget { onContextAction(action) } + targets.append(target) + let item = NSMenuItem(title: title, action: #selector(ClosureMenuItemTarget.invoke), keyEquivalent: "") + item.target = target + item.isEnabled = enabled + if let shortcut { + item.keyEquivalent = String(shortcut.key.character) + item.keyEquivalentModifierMask = nsModifierFlags(from: shortcut.modifiers) + } + menu.addItem(item) + } + + private static func nsModifierFlags(from modifiers: EventModifiers) -> NSEvent.ModifierFlags { + var flags: NSEvent.ModifierFlags = [] + if modifiers.contains(.command) { flags.insert(.command) } + if modifiers.contains(.shift) { flags.insert(.shift) } + if modifiers.contains(.option) { flags.insert(.option) } + if modifiers.contains(.control) { flags.insert(.control) } + if modifiers.contains(.capsLock) { flags.insert(.capsLock) } + if modifiers.contains(.numericPad) { flags.insert(.numericPad) } + // .function is reserved for system applications (macOS 12+ deprecation); menu + // shortcuts never use it in practice, so it's intentionally left unmapped. + return flags + } + + private static func localized(_ key: String, defaultValue: String) -> String { + Bundle.module.localizedString(forKey: key, value: defaultValue, table: nil) + } + + private static func buildMenu( + tab: TabItem, + state: TabContextMenuState, + onContextAction: @escaping (TabContextAction) -> Void, + onApplyTabColor: @escaping (String) -> Void, + retaining targets: inout [ClosureMenuItemTarget] + ) -> NSMenu { + let menu = NSMenu() + menu.autoenablesItems = false + + func add(_ title: String, _ action: TabContextAction, enabled: Bool = true) { + addItem( + to: menu, + title: title, + action: action, + enabled: enabled, + shortcut: state.shortcuts[action], + onContextAction: onContextAction, + retaining: &targets + ) + } + + add("Rename Tab…", .rename) + + if state.hasCustomTitle { + add("Remove Custom Tab Name", .clearName) + } + + menu.addItem(.separator()) + + add("Close Tabs to Left", .closeToLeft, enabled: state.canCloseToLeft) + add("Close Tabs to Right", .closeToRight, enabled: state.canCloseToRight) + add("Close Other Tabs", .closeOthers, enabled: state.canCloseOthers) + add("Move Tab…", .move) + + if state.isTerminal { + add( + localized("command.moveTabToLeftPane.title", defaultValue: "Move to Left Pane"), + .moveToLeftPane, + enabled: state.canMoveToLeftPane + ) + add( + localized("command.moveTabToRightPane.title", defaultValue: "Move to Right Pane"), + .moveToRightPane, + enabled: state.canMoveToRightPane + ) + } + + menu.addItem(.separator()) + + add("New Terminal Tab to Right", .newTerminalToRight) + add("New Browser Tab to Right", .newBrowserToRight) + + if state.isBrowser { + menu.addItem(.separator()) + add("Reload Tab", .reload) + add("Duplicate Tab", .duplicate) + } + + menu.addItem(.separator()) + + if state.hasSplits { + add(state.isZoomed ? "Exit Zoom" : "Zoom Pane", .toggleZoom) + } + + add(state.isPinned ? "Unpin Tab" : "Pin Tab", .togglePin) + + if state.isUnread { + add("Mark Tab as Read", .markAsRead, enabled: state.canMarkAsRead) + } else { + add("Mark Tab as Unread", .markAsUnread, enabled: state.canMarkAsUnread) + } + + menu.addItem(.separator()) + menu.addItem(buildTabColorSubmenu( + state: state, + onContextAction: onContextAction, + onApplyTabColor: onApplyTabColor, + retaining: &targets + )) + + return menu + } + + private static func buildTabColorSubmenu( + state: TabContextMenuState, + onContextAction: @escaping (TabContextAction) -> Void, + onApplyTabColor: @escaping (String) -> Void, + retaining targets: inout [ClosureMenuItemTarget] + ) -> NSMenuItem { + let submenuItem = NSMenuItem( + title: localized("contextMenu.tabColor", defaultValue: "Tab Color"), + action: nil, + keyEquivalent: "" + ) + let submenu = NSMenu() + submenu.autoenablesItems = false + + if state.hasCustomColor { + addItem( + to: submenu, + title: localized("contextMenu.clearColor", defaultValue: "Clear Color"), + action: .clearTabColor, + enabled: true, + shortcut: nil, + onContextAction: onContextAction, + retaining: &targets + ) + } + + addItem( + to: submenu, + title: localized("contextMenu.chooseCustomColor", defaultValue: "Choose Custom Color…"), + action: .chooseCustomTabColor, + enabled: true, + shortcut: nil, + onContextAction: onContextAction, + retaining: &targets + ) + + if !state.colorPalette.isEmpty { + submenu.addItem(.separator()) + } + + for swatch in state.colorPalette { + let target = ClosureMenuItemTarget { onApplyTabColor(swatch.hex) } + targets.append(target) + let item = NSMenuItem(title: swatch.name, action: #selector(ClosureMenuItemTarget.invoke), keyEquivalent: "") + item.target = target + item.isEnabled = true + item.image = swatchImage(color: swatch.swatchColor) + submenu.addItem(item) + } + + submenuItem.submenu = submenu + return submenuItem + } + + private static func swatchImage(color: NSColor, diameter: CGFloat = 12) -> NSImage { + NSImage(size: NSSize(width: diameter, height: diameter), flipped: false) { rect in + color.setFill() + NSBezierPath(ovalIn: rect.insetBy(dx: 0.5, dy: 0.5)).fill() + return true + } + } + } + + func makeCoordinator() -> Coordinator { Coordinator() } + + func makeNSView(context: Context) -> NSView { + let view = NSView(frame: .zero) + view.wantsLayer = true + view.layer?.backgroundColor = NSColor.clear.cgColor + + context.coordinator.view = view + context.coordinator.tab = tab + context.coordinator.contextMenuState = contextMenuState + context.coordinator.onContextAction = onContextAction + context.coordinator.onApplyTabColor = onApplyTabColor + + // Only intercept right-mouse-down and ctrl-left-mouse-down so left click selection, + // middle click close, and drag/reorder all keep working unmodified. + let coordinator = context.coordinator + coordinator.monitor = NSEvent.addLocalMonitorForEvents(matching: [.rightMouseDown, .leftMouseDown]) { [weak coordinator] event in + guard let coordinator, let v = coordinator.view, let w = v.window else { return event } + guard event.window === w else { return event } + + let isRightClick = event.type == .rightMouseDown + let isCtrlClick = event.type == .leftMouseDown && event.modifierFlags.contains(.control) + guard isRightClick || isCtrlClick else { return event } + + let p = v.convert(event.locationInWindow, from: nil) + guard v.bounds.contains(p) else { return event } + + coordinator.popUpMenu(for: event, in: v) + return nil // swallow so it doesn't also select the tab or start a drag + } + + return view + } + + func updateNSView(_ nsView: NSView, context: Context) { + // Refresh on every render so a stale enabled/disabled state or pinned/unread flag + // from a previous render can never be shown in a freshly popped menu. + context.coordinator.view = nsView + context.coordinator.tab = tab + context.coordinator.contextMenuState = contextMenuState + context.coordinator.onContextAction = onContextAction + context.coordinator.onApplyTabColor = onApplyTabColor + } +} diff --git a/vendor/bonsplit/Sources/Bonsplit/Public/BonsplitController.swift b/vendor/bonsplit/Sources/Bonsplit/Public/BonsplitController.swift index a2e2b9215..25e8e0a4e 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Public/BonsplitController.swift +++ b/vendor/bonsplit/Sources/Bonsplit/Public/BonsplitController.swift @@ -116,6 +116,7 @@ public final class BonsplitController { showsNotificationBadge: Bool = false, isLoading: Bool = false, isPinned: Bool = false, + customColorHex: String? = nil, inPane pane: PaneID? = nil ) -> TabID? { let tabId = TabID() @@ -129,7 +130,8 @@ public final class BonsplitController { isDirty: isDirty, showsNotificationBadge: showsNotificationBadge, isLoading: isLoading, - isPinned: isPinned + isPinned: isPinned, + customColorHex: customColorHex ) let targetPane = pane ?? focusedPaneId ?? PaneID(id: internalController.rootNode.allPaneIds.first!.id) @@ -166,7 +168,8 @@ public final class BonsplitController { isDirty: isDirty, showsNotificationBadge: showsNotificationBadge, isLoading: isLoading, - isPinned: isPinned + isPinned: isPinned, + customColorHex: customColorHex ) internalController.addTab(tabItem, toPane: PaneID(id: targetPane.id), atIndex: insertIndex) @@ -188,6 +191,12 @@ public final class BonsplitController { delegate?.splitTabBar(self, didRequestTabContextAction: action, for: tab, inPane: pane) } + /// Request the delegate to apply a palette color to a tab. + public func requestTabColor(_ hex: String, for tabId: TabID, inPane pane: PaneID) { + guard let tab = tab(tabId) else { return } + delegate?.splitTabBar(self, didRequestTabColor: hex, for: tab, inPane: pane) + } + /// Update an existing tab's metadata /// - Parameters: /// - tabId: The tab to update @@ -210,7 +219,8 @@ public final class BonsplitController { isDirty: Bool? = nil, showsNotificationBadge: Bool? = nil, isLoading: Bool? = nil, - isPinned: Bool? = nil + isPinned: Bool? = nil, + customColorHex: String?? = nil ) { guard let (pane, tabIndex) = findTabInternal(tabId) else { return } @@ -241,6 +251,9 @@ public final class BonsplitController { if let isPinned = isPinned { pane.tabs[tabIndex].isPinned = isPinned } + if let customColorHex = customColorHex { + pane.tabs[tabIndex].customColorHex = customColorHex + } } /// Close a tab by ID @@ -652,6 +665,12 @@ public final class BonsplitController { /// Set by the host app to sync with its customizable keyboard shortcut settings. public var contextMenuShortcuts: [TabContextAction: KeyboardShortcut] = [:] + // MARK: - Tab Color Palette + + /// Host-provided palette entries for the tab-color context menu. Resolved at menu-build + /// time (not cached) so it always reflects the host's current colors and appearance. + @ObservationIgnored public var tabColorPaletteProvider: (() -> [TabColorSwatch])? + // MARK: - Query Methods /// Get all tab IDs diff --git a/vendor/bonsplit/Sources/Bonsplit/Public/BonsplitDelegate.swift b/vendor/bonsplit/Sources/Bonsplit/Public/BonsplitDelegate.swift index 95ef1f1ac..0bd03fbf1 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Public/BonsplitDelegate.swift +++ b/vendor/bonsplit/Sources/Bonsplit/Public/BonsplitDelegate.swift @@ -58,6 +58,9 @@ public protocol BonsplitDelegate: AnyObject { /// Called when the user triggers an action from a tab's context menu. func splitTabBar(_ controller: BonsplitController, didRequestTabContextAction action: TabContextAction, for tab: Tab, inPane pane: PaneID) + /// Called when the user picks a palette color for a tab from its context menu. + func splitTabBar(_ controller: BonsplitController, didRequestTabColor hex: String, for tab: Tab, inPane pane: PaneID) + // MARK: - Geometry /// Called when any pane geometry changes (resize, split, close) @@ -83,6 +86,7 @@ public extension BonsplitDelegate { func splitTabBar(_ controller: BonsplitController, didFocusPane pane: PaneID) {} func splitTabBar(_ controller: BonsplitController, didRequestNewTab kind: String, inPane pane: PaneID) {} func splitTabBar(_ controller: BonsplitController, didRequestTabContextAction action: TabContextAction, for tab: Tab, inPane pane: PaneID) {} + func splitTabBar(_ controller: BonsplitController, didRequestTabColor hex: String, for tab: Tab, inPane pane: PaneID) {} func splitTabBar(_ controller: BonsplitController, didChangeGeometry snapshot: LayoutSnapshot) {} func splitTabBar(_ controller: BonsplitController, shouldNotifyDuringDrag: Bool) -> Bool { false } } diff --git a/vendor/bonsplit/Sources/Bonsplit/Public/Types/Tab.swift b/vendor/bonsplit/Sources/Bonsplit/Public/Types/Tab.swift index 70fadf40f..94cedd359 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Public/Types/Tab.swift +++ b/vendor/bonsplit/Sources/Bonsplit/Public/Types/Tab.swift @@ -17,6 +17,8 @@ public struct Tab: Identifiable, Hashable, Sendable { public let isLoading: Bool /// Whether the tab is pinned in its pane. public let isPinned: Bool + /// Optional hex color (e.g. "#1565C0") tinting this tab's background. + public let customColorHex: String? public init( id: TabID = TabID(), @@ -28,7 +30,8 @@ public struct Tab: Identifiable, Hashable, Sendable { isDirty: Bool = false, showsNotificationBadge: Bool = false, isLoading: Bool = false, - isPinned: Bool = false + isPinned: Bool = false, + customColorHex: String? = nil ) { self.id = id self.title = title @@ -40,6 +43,7 @@ public struct Tab: Identifiable, Hashable, Sendable { self.showsNotificationBadge = showsNotificationBadge self.isLoading = isLoading self.isPinned = isPinned + self.customColorHex = customColorHex } internal init(from tabItem: TabItem) { @@ -53,5 +57,6 @@ public struct Tab: Identifiable, Hashable, Sendable { self.showsNotificationBadge = tabItem.showsNotificationBadge self.isLoading = tabItem.isLoading self.isPinned = tabItem.isPinned + self.customColorHex = tabItem.customColorHex } } diff --git a/vendor/bonsplit/Sources/Bonsplit/Public/Types/TabContextAction.swift b/vendor/bonsplit/Sources/Bonsplit/Public/Types/TabContextAction.swift index 47ec09d32..4bdf13a78 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Public/Types/TabContextAction.swift +++ b/vendor/bonsplit/Sources/Bonsplit/Public/Types/TabContextAction.swift @@ -1,4 +1,18 @@ import Foundation +import AppKit + +/// A single swatch entry for the tab-color context menu, supplied by the host app. +public struct TabColorSwatch { + public let name: String + public let hex: String + public let swatchColor: NSColor + + public init(name: String, hex: String, swatchColor: NSColor) { + self.name = name + self.hex = hex + self.swatchColor = swatchColor + } +} /// Context menu actions that can be triggered from a tab item. public enum TabContextAction: String, CaseIterable, Sendable { @@ -18,4 +32,6 @@ public enum TabContextAction: String, CaseIterable, Sendable { case markAsRead case markAsUnread case toggleZoom + case clearTabColor + case chooseCustomTabColor } diff --git a/vendor/bonsplit/Sources/Bonsplit/Resources/en.lproj/Localizable.strings b/vendor/bonsplit/Sources/Bonsplit/Resources/en.lproj/Localizable.strings index 89fcf28af..fbe1639b1 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Resources/en.lproj/Localizable.strings +++ b/vendor/bonsplit/Sources/Bonsplit/Resources/en.lproj/Localizable.strings @@ -1,2 +1,5 @@ "command.moveTabToLeftPane.title" = "Move to Left Pane"; "command.moveTabToRightPane.title" = "Move to Right Pane"; +"contextMenu.tabColor" = "Tab Color"; +"contextMenu.clearColor" = "Clear Color"; +"contextMenu.chooseCustomColor" = "Choose Custom Color…"; diff --git a/vendor/bonsplit/Sources/Bonsplit/Resources/ja.lproj/Localizable.strings b/vendor/bonsplit/Sources/Bonsplit/Resources/ja.lproj/Localizable.strings index d272f84f4..eb9a47bc7 100644 --- a/vendor/bonsplit/Sources/Bonsplit/Resources/ja.lproj/Localizable.strings +++ b/vendor/bonsplit/Sources/Bonsplit/Resources/ja.lproj/Localizable.strings @@ -1,2 +1,5 @@ "command.moveTabToLeftPane.title" = "左のペインへ移動"; "command.moveTabToRightPane.title" = "右のペインへ移動"; +"contextMenu.tabColor" = "タブの色"; +"contextMenu.clearColor" = "色をクリア"; +"contextMenu.chooseCustomColor" = "カスタムカラーを選択…"; From 2f2dcaa1d68e3bd33433f6ffbcc99b6e00528d2c Mon Sep 17 00:00:00 2001 From: arzafran Date: Mon, 28 Sep 2026 11:17:24 -0300 Subject: [PATCH 14/19] refactor: use generated v2 method names in the MCP server --- CLI-MCP/MCPSocketBridge.swift | 8 +- CLI-MCP/Resources/SurfaceTextResource.swift | 2 +- CLI-MCP/Resources/TreeResource.swift | 2 +- CLI-MCP/Tools/BrowserTools.swift | 164 ++++++++++---------- CLI-MCP/Tools/FocusTools.swift | 26 ++-- CLI-MCP/Tools/LayoutTools.swift | 6 +- CLI-MCP/Tools/NotificationTools.swift | 10 +- CLI-MCP/Tools/PaneTools.swift | 14 +- CLI-MCP/Tools/ReviewTools.swift | 10 +- CLI-MCP/Tools/SnapshotTools.swift | 4 +- CLI-MCP/Tools/SurfaceTools.swift | 58 +++---- CLI-MCP/Tools/SystemTools.swift | 8 +- CLI-MCP/Tools/WindowTools.swift | 8 +- CLI-MCP/Tools/WorkspaceTools.swift | 60 +++---- CLI-MCP/Tools/WorktreeTools.swift | 6 +- GhosttyTabs.xcodeproj/project.pbxproj | 2 + 16 files changed, 195 insertions(+), 193 deletions(-) diff --git a/CLI-MCP/MCPSocketBridge.swift b/CLI-MCP/MCPSocketBridge.swift index 7e9fd1b04..52e70c2c5 100644 --- a/CLI-MCP/MCPSocketBridge.swift +++ b/CLI-MCP/MCPSocketBridge.swift @@ -113,7 +113,7 @@ struct MCPSocketBridge { if let socketPassword { _ = try Self.sendRequest( - method: "auth.login", + method: V2MethodNames.authLogin, params: ["password": socketPassword], socketFD: socketFD ) @@ -169,11 +169,11 @@ struct MCPSocketBridge { static func responseTimeout(method: String, params: [String: Any]) -> TimeInterval { let timeoutMs: Int switch method { - case "surface.wait": + case V2MethodNames.surfaceWait: timeoutMs = timeoutInteger(params["timeout_ms"]) ?? timeoutInteger(params["timeout"]) ?? 30_000 - case "browser.wait": + case V2MethodNames.browserWait: timeoutMs = timeoutInteger(params["timeout_ms"]) ?? 5_000 - case "browser.download.wait": + case V2MethodNames.browserDownloadWait: timeoutMs = timeoutInteger(params["timeout_ms"]) ?? timeoutInteger(params["timeout"]) ?? 10_000 default: return defaultResponseTimeoutSeconds diff --git a/CLI-MCP/Resources/SurfaceTextResource.swift b/CLI-MCP/Resources/SurfaceTextResource.swift index 7e7fd30e9..6b662b3f5 100644 --- a/CLI-MCP/Resources/SurfaceTextResource.swift +++ b/CLI-MCP/Resources/SurfaceTextResource.swift @@ -30,7 +30,7 @@ enum SurfaceTextResource { let bridge = MCPSocketBridge() do { - let result = try bridge.send(method: "surface.read_text", params: params) + let result = try bridge.send(method: V2MethodNames.surfaceReadText, params: params) let text = (result["text"] as? String) ?? "" return [.text(text, uri: "programa://surface/\(surfaceId)/text", mimeType: "text/plain")] } catch { diff --git a/CLI-MCP/Resources/TreeResource.swift b/CLI-MCP/Resources/TreeResource.swift index ba2c14c67..3b2e76c6d 100644 --- a/CLI-MCP/Resources/TreeResource.swift +++ b/CLI-MCP/Resources/TreeResource.swift @@ -30,7 +30,7 @@ enum TreeResource { let bridge = MCPSocketBridge() do { - let result = try bridge.send(method: "system.tree", params: params) + let result = try bridge.send(method: V2MethodNames.systemTree, params: params) return [try ResourceCatalog.jsonContent(for: result, uri: uri)] } catch { throw MCPErrorMapping.mcpError(for: error) diff --git a/CLI-MCP/Tools/BrowserTools.swift b/CLI-MCP/Tools/BrowserTools.swift index fa48dc6ea..0850252fd 100644 --- a/CLI-MCP/Tools/BrowserTools.swift +++ b/CLI-MCP/Tools/BrowserTools.swift @@ -47,7 +47,7 @@ enum BrowserTools { static let tools: [ProgramaTool] = [ ProgramaTool( name: "browser_open_split", - socketMethod: "browser.open_split", + socketMethod: V2MethodNames.browserOpenSplit, description: "Opens a new browser surface as a split next to a source surface (or reuses an existing sibling browser pane when one is already positioned to the right). Never raises/activates the Programa window or switches the selected workspace; inside the target workspace the new tab becomes that workspace's focused surface, so the user only sees a change if they are already looking at that workspace. Returns the new surface_id.", inputSchema: ProgramaToolSchema.object(properties: [ "url": ProgramaToolSchema.string("Initial URL to load. Opens a blank browser tab if omitted."), @@ -59,7 +59,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_navigate", - socketMethod: "browser.navigate", + socketMethod: V2MethodNames.browserNavigate, description: "Navigates an existing browser surface to a URL.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -73,7 +73,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_back", - socketMethod: "browser.back", + socketMethod: V2MethodNames.browserBack, description: "Navigates a browser surface back one entry in its history.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -86,7 +86,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_forward", - socketMethod: "browser.forward", + socketMethod: V2MethodNames.browserForward, description: "Navigates a browser surface forward one entry in its history.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -99,7 +99,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_reload", - socketMethod: "browser.reload", + socketMethod: V2MethodNames.browserReload, description: "Reloads a browser surface's current page.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -112,7 +112,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_url_get", - socketMethod: "browser.url.get", + socketMethod: V2MethodNames.browserUrlGet, description: "Returns a browser surface's current URL.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -125,7 +125,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_is_webview_focused", - socketMethod: "browser.is_webview_focused", + socketMethod: V2MethodNames.browserIsWebviewFocused, description: "Reports whether keyboard focus is currently inside a browser surface's web view. Read-only; does not move focus.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -138,7 +138,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_snapshot", - socketMethod: "browser.snapshot", + socketMethod: V2MethodNames.browserSnapshot, description: "Returns an accessibility-tree-style snapshot of the page (roles, names, and element refs usable by selector-based tools), plus title/url/ready_state and the page text/HTML.", inputSchema: ProgramaToolSchema.object(properties: [ "interactive": ProgramaToolSchema.boolean("If true, include only interactive elements (links, buttons, inputs, etc). Defaults to false (include the full tree)."), @@ -153,7 +153,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_eval", - socketMethod: "browser.eval", + socketMethod: V2MethodNames.browserEval, description: "Runs arbitrary JavaScript in the browser surface's page context and returns its result (JSON-normalized).", inputSchema: ProgramaToolSchema.object( properties: [ @@ -167,7 +167,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_wait", - socketMethod: "browser.wait", + socketMethod: V2MethodNames.browserWait, description: "Waits (up to a timeout) for a condition to become true: a selector to appear, the URL to contain a substring, the page text to contain a substring, a document.readyState value, or a custom JS boolean expression. Provide at most one condition; defaults to waiting for document.readyState === 'complete'.", inputSchema: ProgramaToolSchema.object(properties: [ "timeout_ms": ProgramaToolSchema.integer("Maximum time to wait, in milliseconds. Defaults to 5000."), @@ -183,7 +183,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_click", - socketMethod: "browser.click", + socketMethod: V2MethodNames.browserClick, description: "Clicks the element matched by a selector (scrolls it into view first, dispatches a real click event or calls .click()).", inputSchema: ProgramaToolSchema.object( properties: [ @@ -198,7 +198,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_dblclick", - socketMethod: "browser.dblclick", + socketMethod: V2MethodNames.browserDblclick, description: "Double-clicks the element matched by a selector.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -213,7 +213,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_hover", - socketMethod: "browser.hover", + socketMethod: V2MethodNames.browserHover, description: "Hovers the element matched by a selector (dispatches mouseover/mouseenter events).", inputSchema: ProgramaToolSchema.object( properties: [ @@ -228,7 +228,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_type", - socketMethod: "browser.type", + socketMethod: V2MethodNames.browserType, description: "Appends text to the element matched by a selector (focuses it, then appends to its value/textContent and fires input/change events). Use browser_fill to replace the value instead of appending.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -244,7 +244,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_fill", - socketMethod: "browser.fill", + socketMethod: V2MethodNames.browserFill, description: "Sets the element matched by a selector to an exact value (replacing any existing value) and fires input/change events. Accepts an empty string to clear the field.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -261,7 +261,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_press", - socketMethod: "browser.press", + socketMethod: V2MethodNames.browserPress, description: "Dispatches a keydown, keypress, and keyup for a key to the page's currently focused element (or body if none).", inputSchema: ProgramaToolSchema.object( properties: [ @@ -275,7 +275,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_keydown", - socketMethod: "browser.keydown", + socketMethod: V2MethodNames.browserKeydown, description: "Dispatches only a keydown event for a key to the page's currently focused element (or body if none). Use browser_press for a full keydown/keypress/keyup sequence.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -289,7 +289,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_keyup", - socketMethod: "browser.keyup", + socketMethod: V2MethodNames.browserKeyup, description: "Dispatches only a keyup event for a key to the page's currently focused element (or body if none).", inputSchema: ProgramaToolSchema.object( properties: [ @@ -303,7 +303,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_check", - socketMethod: "browser.check", + socketMethod: V2MethodNames.browserCheck, description: "Sets a checkbox/radio element matched by a selector to checked, firing input/change events. Fails if the element has no 'checked' property.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -318,7 +318,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_uncheck", - socketMethod: "browser.uncheck", + socketMethod: V2MethodNames.browserUncheck, description: "Sets a checkbox/radio element matched by a selector to unchecked, firing input/change events. Fails if the element has no 'checked' property.", inputSchema: ProgramaToolSchema.object( properties: [ @@ -333,7 +333,7 @@ enum BrowserTools { ), ProgramaTool( name: "browser_select", - socketMethod: "browser.select", + socketMethod: V2MethodNames.browserSelect, description: "Sets a