Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ DerivedData/
*.xcuserstate
xcuserdata/

# Development plans
PLAN.md

# macOS
.DS_Store

Expand Down
17 changes: 17 additions & 0 deletions Resources/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -87391,6 +87391,23 @@
}
}
}
},
"tab.group.defaultName": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Group"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "グループ"
}
}
}
}
}
}
10 changes: 10 additions & 0 deletions Sources/SessionPersistence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,19 @@ enum SessionSplitOrientation: String, Codable, Sendable {
}
}

struct SessionTabGroupSnapshot: Codable, Sendable {
var id: UUID
var name: String
var colorHex: String
var isCollapsed: Bool?
}

struct SessionPaneLayoutSnapshot: Codable, Sendable {
var panelIds: [UUID]
var selectedPanelId: UUID?
var groups: [SessionTabGroupSnapshot]? = nil
/// Maps panel ID → group ID for tabs assigned to a group.
var panelGroupAssignments: [UUID: UUID]? = nil
}

struct SessionSplitLayoutSnapshot: Codable, Sendable {
Expand Down
135 changes: 134 additions & 1 deletion Sources/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,16 @@ extension Workspace {
applyProcessTitle(snapshot.processTitle)
setCustomTitle(snapshot.customTitle)
setCustomDescription(snapshot.customDescription)

// Before syncing workspace color, adopt any workspace group restored from
// the snapshot so syncWorkspaceGroup updates it instead of creating a duplicate.
if let colorHex = snapshot.customColor?.replacingOccurrences(of: "#", with: ""),
let firstPane = bonsplitController.allPaneIds.first {
let restored = bonsplitController.groups(inPane: firstPane)
if let match = restored.first(where: { $0.colorHex == colorHex }) {
workspaceGroupId = match.id
}
}
setCustomColor(snapshot.customColor)
isPinned = snapshot.isPinned

Expand Down Expand Up @@ -374,10 +384,31 @@ extension Workspace {
case .pane(let pane):
let panelIds = sessionPanelIDs(for: pane)
let selectedPanelId = pane.selectedTabId.flatMap(sessionPanelID(forExternalTabIDString:))

// Capture all tab groups for this pane (including workspace-managed groups)
let paneId = PaneID(id: UUID(uuidString: pane.id) ?? UUID())
let paneGroups = bonsplitController.groups(inPane: paneId)
let groupSnapshots: [SessionTabGroupSnapshot]? = paneGroups.isEmpty ? nil : paneGroups.map {
SessionTabGroupSnapshot(id: $0.id, name: $0.name, colorHex: $0.colorHex, isCollapsed: $0.isCollapsed ? true : nil)
}

var panelGroupAssignments: [UUID: UUID]? = nil
let tabs = bonsplitController.tabs(inPane: paneId)
let assignments = tabs.compactMap { tab -> (UUID, UUID)? in
guard let gid = tab.groupId,
let panelId = panelIdFromSurfaceId(tab.id) else { return nil }
return (panelId, gid)
}
if !assignments.isEmpty {
panelGroupAssignments = Dictionary(assignments, uniquingKeysWith: { _, last in last })
}

return .pane(
SessionPaneLayoutSnapshot(
panelIds: panelIds,
selectedPanelId: selectedPanelId
selectedPanelId: selectedPanelId,
groups: groupSnapshots,
panelGroupAssignments: panelGroupAssignments
)
)
case .split(let split):
Expand Down Expand Up @@ -634,6 +665,27 @@ extension Workspace {
bonsplitController.focusPane(paneId)
bonsplitController.selectTab(selectedTabId)
}

// Restore tab groups
if let savedGroups = snapshot.groups {
for groupSnapshot in savedGroups {
bonsplitController.createGroup(
id: groupSnapshot.id,
name: groupSnapshot.name,
colorHex: groupSnapshot.colorHex,
isCollapsed: groupSnapshot.isCollapsed ?? false,
inPane: paneId
)
}

if let assignments = snapshot.panelGroupAssignments {
for (oldPanelId, groupId) in assignments {
guard let newPanelId = oldToNewPanelIds[oldPanelId],
let tabId = surfaceIdFromPanelId(newPanelId) else { continue }
bonsplitController.assignTab(tabId, toGroup: groupId)
}
}
}
}

private func createPanel(from snapshot: SessionPanelSnapshot, inPane paneId: PaneID) -> UUID? {
Expand Down Expand Up @@ -6486,6 +6538,8 @@ final class Workspace: Identifiable, ObservableObject {
@Published var customDescription: String?
@Published var isPinned: Bool = false
@Published var customColor: String? // hex string, e.g. "#C0392B"
/// Group ID for the auto-created workspace group (ties sidebar color to tab groups).
var workspaceGroupId: UUID?
@Published var currentDirectory: String
private(set) var preferredBrowserProfileID: UUID?

Expand Down Expand Up @@ -7520,6 +7574,51 @@ final class Workspace: Identifiable, ObservableObject {
} else {
customColor = nil
}
syncWorkspaceGroup()
}

/// Ensures the workspace group exists when the workspace has a color, and
/// auto-assigns all tabs in every pane to the workspace group.
func syncWorkspaceGroup() {
guard let colorHex = customColor?.replacingOccurrences(of: "#", with: "") else {
// No color → remove workspace group if it exists
if let gid = workspaceGroupId {
for paneId in bonsplitController.allPaneIds {
bonsplitController.deleteGroup(gid, inPane: paneId)
}
workspaceGroupId = nil
}
return
}

let groupName = customTitle ?? title
if let gid = workspaceGroupId {
// Update existing group color and name in all panes
for paneId in bonsplitController.allPaneIds {
// Ensure the group exists in this pane (may be new from a split)
if bonsplitController.groups(inPane: paneId).first(where: { $0.id == gid }) == nil {
bonsplitController.createGroup(id: gid, name: groupName, colorHex: colorHex, inPane: paneId)
for tab in bonsplitController.tabs(inPane: paneId) where tab.groupId == nil {
bonsplitController.assignTab(tab.id, toGroup: gid)
}
} else {
bonsplitController.renameGroup(gid, to: groupName, inPane: paneId)
bonsplitController.changeGroupColor(gid, to: colorHex, inPane: paneId)
}
}
} else {
// Create workspace group with a single stable ID across all panes.
// Only assign tabs that aren't already in a group (preserves saved assignments
// during restore and user-created groups during normal operation).
let stableId = UUID()
workspaceGroupId = stableId
for paneId in bonsplitController.allPaneIds {
bonsplitController.createGroup(id: stableId, name: groupName, colorHex: colorHex, inPane: paneId)
for tab in bonsplitController.tabs(inPane: paneId) where tab.groupId == nil {
bonsplitController.assignTab(tab.id, toGroup: stableId)
}
}
}
}
Comment on lines +7589 to 7622
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Workspace group UUID diverges across panes in split workspaces

createGroup(name:colorHex:inPane:) is called once per pane and each call returns a fresh UUID. Only the first pane's UUID is stored in workspaceGroupId (guarded by if workspaceGroupId == nil). Every subsequent deleteGroup, renameGroup, and changeGroupColor call passes that first-pane UUID to all panes — for pane 2 and beyond the group is never found and operations silently no-op. The didCreateTab delegate has the same flaw: it assigns a newly created tab to the first-pane UUID even when the tab lives in a different pane.

Fix: use a per-pane map ([PaneID: UUID]) instead of a single workspaceGroupId, or pass the desired UUID into createGroup using the id-bearing overload (already present for restore) so all panes share one stable UUID.


private static func normalizedCustomDescription(_ description: String?) -> String? {
Expand All @@ -7540,6 +7639,13 @@ final class Workspace: Identifiable, ObservableObject {
customTitle = trimmed
self.title = trimmed
}
// Keep workspace group name in sync with workspace title
if let gid = workspaceGroupId {
let groupName = customTitle ?? self.title
for paneId in bonsplitController.allPaneIds {
bonsplitController.renameGroup(gid, to: groupName, inPane: paneId)
}
}
}

func setCustomDescription(_ description: String?) {
Expand Down Expand Up @@ -12131,6 +12237,19 @@ extension Workspace: BonsplitDelegate {
}
}

func splitTabBar(_ controller: BonsplitController, didCreateTab tab: Bonsplit.Tab, inPane pane: PaneID) {
// Auto-assign new tabs to the workspace group when the workspace has a color.
// Ensure the group exists in this pane first (may be new from a split).
if let gid = workspaceGroupId, tab.groupId == nil {
if bonsplitController.groups(inPane: pane).first(where: { $0.id == gid }) == nil {
let groupName = customTitle ?? title
let colorHex = customColor?.replacingOccurrences(of: "#", with: "") ?? ""
bonsplitController.createGroup(id: gid, name: groupName, colorHex: colorHex, inPane: pane)
}
bonsplitController.assignTab(tab.id, toGroup: gid)
}
}

func splitTabBar(_ controller: BonsplitController, didRequestNewTab kind: String, inPane pane: PaneID) {
switch kind {
case "terminal":
Expand Down Expand Up @@ -12180,6 +12299,20 @@ extension Workspace: BonsplitDelegate {
case .toggleZoom:
guard let panelId = panelIdFromSurfaceId(tab.id) else { return }
toggleSplitZoom(panelId: panelId)
case .newGroup:
let colors = ["4F46E5", "DC2626", "059669", "D97706", "7C3AED", "DB2777"]
let existingCount = bonsplitController.groups(inPane: pane).count
let color = colors[existingCount % colors.count]
let groupNumber = existingCount + 1
let name = String(localized: "tab.group.defaultName", defaultValue: "Group") + " \(groupNumber)"
if let groupId = bonsplitController.createGroup(name: name, colorHex: color, inPane: pane) {
bonsplitController.assignTab(tab.id, toGroup: groupId)
}
Comment on lines +12302 to +12310
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Localize the default group name.

"Group \(existingCount + 1)" is user-facing and bypasses the new Japanese localization work. Please switch it to String(localized:..., defaultValue: ...) and add the key to Resources/Localizable.xcstrings. As per coding guidelines, "All user-facing strings must be localized using String(localized: "key.name", defaultValue: "English text") for every string shown in the UI."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Workspace.swift` around lines 12252 - 12259, Replace the hard-coded
user-facing name "Group \(existingCount + 1)" in the .newGroup case with a
localized lookup using String(localized: "group.defaultName", defaultValue:
"Group %d") (format with existingCount + 1 when passing into createGroup),
updating the name variable assignment used with
bonsplitController.createGroup(name: ..., colorHex: ..., inPane: pane) and
preserving existing behavior; also add the "group.defaultName" key to
Resources/Localizable.xcstrings with the English default "Group %d" and provide
the Japanese translation entry in the same file.

case .addToGroup:
// Handled via direct bonsplitController.assignTab() from the submenu
Comment on lines +12311 to +12312
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Non-localized group name violates localization policy

The default group name "Group \(existingCount + 1)" is a bare string literal shown directly as a user-visible tab group title. CLAUDE.md requires every user-facing string to use String(localized:defaultValue:) with a key in Resources/Localizable.xcstrings (English + Japanese).

Suggested change
case .addToGroup:
// Handled via direct bonsplitController.assignTab() from the submenu
let name = String(localized: "tab.group.defaultName", defaultValue: "Group \(existingCount + 1)")

break
case .removeFromGroup:
bonsplitController.removeTabFromGroup(tab.id)
@unknown default:
Comment on lines +12314 to 12316
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 .addToGroup context action is a silent no-op

The .addToGroup case breaks without calling bonsplitController.assignTab. The comment claims assignment is "handled via direct bonsplitController.assignTab() from the submenu", but no such call site appears in this diff. If the submenu sends a TabContextAction.addToGroup delegate callback, the tab assignment will be silently dropped here.

break
}
Expand Down