Skip to content
Merged
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
50 changes: 43 additions & 7 deletions Packages/macOS/CmuxNext/Sources/CmuxNextHomeEngine/HomeDock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public final class HomeDock {
/// ever (``placeholderCheckedKey``), so a shell the person opens there later, works in and
/// leaves at `~` is never closed on a reconnect or a later launch.
private var placeholderChecked = false
/// The allow-tabs step waiting for the dock's resource ids (``allowTabsWhenIdentified``).
private var allowTabsWait: Task<Void, Never>?
private let defaults: UserDefaults
/// Set once the first launch with `pinned-dock-alone-v1` looked for the placeholder shell.
static let placeholderCheckedKey = "cmuxNext.home.placeholderShellChecked"
Expand Down Expand Up @@ -179,17 +181,31 @@ public final class HomeDock {
/// ``allowTabs``, with a failure logged and returned as the step, so the rest of the connect
/// (the placeholder check) still runs; the next connect tries again.
private func allowTabsLogged(_ connection: DaemonConnection, store: DaemonStore,
workspace: @MainActor @Sendable () -> WorkspaceModel?, fresh: Bool) async -> String? {
workspace: @escaping @MainActor @Sendable () -> WorkspaceModel?, fresh: Bool) async -> String? {
await allowTabsLogged(store: store, workspace: workspace, fresh: fresh) { screen, pane in
try await connection.setColumnAllowsTabs(true, screen: screen, pane: pane)
}
}

/// The `column.update {allow_tabs: true}` of the dock column on `screen` holding `pane` (tests pass their own).
typealias AllowTabs = @Sendable (_ screen: ResourceID, _ pane: ResourceID) async throws -> Void

/// ``allowTabsLogged(_:store:workspace:fresh:)`` with the request as `allow`.
func allowTabsLogged(store: DaemonStore, workspace: @escaping @MainActor @Sendable () -> WorkspaceModel?, fresh: Bool,
allow: @escaping AllowTabs) async -> String? {
do {
return try await allowTabs(connection, store: store, workspace: workspace, fresh: fresh)
return try await allowTabs(store: store, workspace: workspace, fresh: fresh, allow: allow)
} catch {
logger.error("home dock allow_tabs: \(String(describing: error), privacy: .public)")
return "home dock allow_tabs failed: \(String(describing: error))"
}
}

private func allowTabs(_ connection: DaemonConnection, store: DaemonStore,
workspace: @MainActor @Sendable () -> WorkspaceModel?, fresh: Bool) async throws -> String? {
private func allowTabs(store: DaemonStore, workspace: @escaping @MainActor @Sendable () -> WorkspaceModel?, fresh: Bool,
allow: @escaping AllowTabs) async throws -> String? {
// This step (a connect's, or the wait's own) supersedes a wait for the ids.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This cancels the pending wait before the guard decides whether this step does anything. A connect that returns nil here, such as one with fresh false after the key is set, still drops a wait that was created with fresh true and would have sent the update. Move the cancellation after the guard, or only cancel when this call proceeds to the ids check.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextHomeEngine/HomeDock.swift, line 206:

<comment>This cancels the pending wait before the guard decides whether this step does anything. A connect that returns nil here, such as one with `fresh` false after the key is set, still drops a wait that was created with `fresh` true and would have sent the update. Move the cancellation after the guard, or only cancel when this call proceeds to the ids check.</comment>

<file context>
@@ -179,17 +181,31 @@ public final class HomeDock {
-                           workspace: @MainActor @Sendable () -> WorkspaceModel?, fresh: Bool) async throws -> String? {
+    private func allowTabs(store: DaemonStore, workspace: @escaping @MainActor @Sendable () -> WorkspaceModel?, fresh: Bool,
+                           allow: @escaping AllowTabs) async throws -> String? {
+        // This step (a connect's, or the wait's own) supersedes a wait for the ids.
+        allowTabsWait?.cancel()
+        allowTabsWait = nil
</file context>

allowTabsWait?.cancel()
allowTabsWait = nil
guard store.supports(DaemonCapability.pinnedDock.rawValue), fresh || !defaults.bool(forKey: Self.dockTakesTabsKey),
let placed = workspace().flatMap(Self.pageTab(in:)),
let dock = Self.dockColumn(placed.pane, on: placed.screen)?.dock else { return nil }
Expand All @@ -199,15 +215,35 @@ public final class HomeDock {
return nil
}
guard let screen = placed.screen.resourceID, let pane = placed.pane.resourceID else {
// The key stays unset, so the next connect tries again.
logger.error("home dock allow_tabs: the tree has no resource id for the dock's screen or pane yet")
// The step runs again when the tree first reports the ids (FC-1: it waited for the next connect, and the
// dock took no tab for the whole launch). Event-driven: the store's records, no timer.
allowTabsWhenIdentified(store: store, workspace: workspace, fresh: fresh, allow: allow)
return "home dock allow_tabs waits for resource ids"
}
try await connection.setColumnAllowsTabs(true, screen: screen, pane: pane)
try await allow(screen, pane)
defaults.set(true, forKey: Self.dockTakesTabsKey)
return "home dock takes tabs"
}

/// Runs the allow-tabs step again once Home's page is in a dock whose screen and pane have their resource ids, or
/// stops when the page is gone. One wait at a time: a later step (a connect) replaces it.
private func allowTabsWhenIdentified(store: DaemonStore, workspace: @escaping @MainActor @Sendable () -> WorkspaceModel?,
fresh: Bool, allow: @escaping AllowTabs) {
allowTabsWait?.cancel()
// task-owner: `allowTabsWait`, replaced by the next step and ended when the ids come or the page goes;
// event-driven (Observation of the store's records), no timer.
allowTabsWait = Task { [weak self] in
let identified: @MainActor @Sendable () -> Bool? = {
guard let placed = workspace().flatMap(Self.pageTab(in:)) else { return false }
return placed.screen.resourceID != nil && placed.pane.resourceID != nil ? true : nil
}
guard await self?.shown(identified) == true, !Task.isCancelled, let self else { return }
Comment on lines +235 to +240

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: WindowHomes.ensureDock creates HomeDock() temporarily, so this weak capture can find self == nil after ensure returns and abandon the ID wait. Keep the dock alive until this task finishes.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextHomeEngine/HomeDock.swift, line 235:

<comment>`WindowHomes.ensureDock` creates `HomeDock()` temporarily, so this weak capture can find `self == nil` after `ensure` returns and abandon the ID wait. Keep the dock alive until this task finishes.</comment>

<file context>
@@ -199,15 +215,35 @@ public final class HomeDock {
+        allowTabsWait?.cancel()
+        // task-owner: `allowTabsWait`, replaced by the next step and ended when the ids come or the page goes;
+        // event-driven (Observation of the store's records), no timer.
+        allowTabsWait = Task { [weak self] in
+            let identified: @MainActor @Sendable () -> Bool? = {
+                guard let placed = workspace().flatMap(Self.pageTab(in:)) else { return false }
</file context>
Suggested change
allowTabsWait = Task { [weak self] in
let identified: @MainActor @Sendable () -> Bool? = {
guard let placed = workspace().flatMap(Self.pageTab(in:)) else { return false }
return placed.screen.resourceID != nil && placed.pane.resourceID != nil ? true : nil
}
guard await self?.shown(identified) == true, !Task.isCancelled, let self else { return }
allowTabsWait = Task { [self] in
let identified: @MainActor @Sendable () -> Bool? = {
guard let placed = workspace().flatMap(Self.pageTab(in:)) else { return false }
return placed.screen.resourceID != nil && placed.pane.resourceID != nil ? true : nil
}
guard await shown(identified) == true, !Task.isCancelled else { return }

allowTabsWait = nil // the step below must not cancel the task it runs in
let step = await allowTabsLogged(store: store, workspace: workspace, fresh: fresh, allow: allow)
logger.notice("home dock allow_tabs once identified: \(step ?? "nothing to do", privacy: .public)")
}
}

/// A home restored from before `pinned-dock-alone-v1` kept the terminal the dock move left on
/// its right. A one-time migration: the first time a launch sees the docked Home, when that
/// column holds nothing but such a shell (``isPlaceholderShell``), it closes and the dock stands
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
@testable import CmuxNextDaemon
@testable import CmuxNextHomeEngine
import Foundation
import Testing

/// FC-1 follow-up (fuzz catalog, 2026-11-01): Home's dock is pinned in its dock move and lets other tabs in only by a
/// later `column.update {allow_tabs: true}` (``HomeDock``'s allow-tabs step). That step needs the dock's screen and
/// pane resource ids. When the tree had none yet, the step returned "waits for resource ids" and nothing ran it again
/// until the next connection: the dock took no tab for the whole launch, so every new tab aimed at it was refused
/// (`dock-column-pinned`). Once the tree reports the ids, the step must run, with no timer and no new connect.
@MainActor @Suite(.timeLimit(.minutes(1)))
struct HomeDockAllowTabsTests {
private static let pane = PaneID(rawValue: 1)
private static let workspaceResource = ResourceID(rawValue: "ws_home")
private static let screenResource = ResourceID(rawValue: "scr_home")
private static let paneResource = ResourceID(rawValue: "pane_home")

/// The home workspace with its page tab in a pinned left chat dock that takes no tabs; with the screen and pane
/// resource ids only when `identified`.
private static func tree(identified: Bool) -> DaemonTree {
var page = TabSnapshot(surface: SurfaceID(rawValue: 2), kind: .conversation)
page.conversation = ConversationTabRef(page: WorkspaceModel.homeDockPage)
let dock = DockSnapshot(edge: .left, mode: .docked, role: .agentChat, isPinned: true, allowsTabs: false)
let screen = ScreenSnapshot(id: ScreenID(rawValue: 3), resourceID: identified ? screenResource : nil, active: true,
activePane: pane, layout: .leaf(pane),
columns: [ColumnSnapshot(id: 10, width: 0.3, layout: .leaf(pane), dock: dock)],
panes: [PaneSnapshot(id: pane, resourceID: identified ? paneResource : nil, tabs: [page])])
return DaemonTree(workspaces: [WorkspaceSnapshot(id: WorkspaceHandle(rawValue: 4), key: nil, resourceID: workspaceResource,
name: "Home", screens: [screen])])
}

/// The requests the step sent, read without awaiting it.
@MainActor final class Sent {
var requests: [(screen: ResourceID, pane: ResourceID)] = []
}

@Test func theDockTakesTabsOnceTheTreeReportsItsIDs() async throws {
let store = DaemonStore()
store.noteHandshake(DaemonIdentity(capabilities: [DaemonCapability.pinnedDock.rawValue], generation: "home-dock-allow-tabs"))
store.apply(snapshot: Self.tree(identified: false))
let defaults = try #require(UserDefaults(suiteName: "home-dock-allow-tabs-\(UUID().uuidString)"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: This named UserDefaults suite is never cleaned up, so every test run leaves a persistent domain behind. Keep the suite name in a variable and add defer { defaults.removePersistentDomain(forName: suite) }.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Tests/CmuxNextHomeEngineTests/HomeDockAllowTabsTests.swift, line 41:

<comment>This named `UserDefaults` suite is never cleaned up, so every test run leaves a persistent domain behind. Keep the suite name in a variable and add `defer { defaults.removePersistentDomain(forName: suite) }`.</comment>

<file context>
@@ -0,0 +1,59 @@
+        let store = DaemonStore()
+        store.noteHandshake(DaemonIdentity(capabilities: [DaemonCapability.pinnedDock.rawValue], generation: "home-dock-allow-tabs"))
+        store.apply(snapshot: Self.tree(identified: false))
+        let defaults = try #require(UserDefaults(suiteName: "home-dock-allow-tabs-\(UUID().uuidString)"))
+        let dock = HomeDock(defaults: defaults)
+        let sent = Sent()
</file context>
Suggested change
let defaults = try #require(UserDefaults(suiteName: "home-dock-allow-tabs-\(UUID().uuidString)"))
let suite = "home-dock-allow-tabs-\(UUID().uuidString)"
let defaults = try #require(UserDefaults(suiteName: suite))
defer { defaults.removePersistentDomain(forName: suite) }

let dock = HomeDock(defaults: defaults)
let sent = Sent()
let workspace: @MainActor @Sendable () -> WorkspaceModel? = { store.workspaces.first { $0.resourceID == Self.workspaceResource } }
let step = await dock.allowTabsLogged(store: store, workspace: workspace, fresh: true) { screen, pane in
await MainActor.run { sent.requests.append((screen, pane)) }
}
#expect(step == "home dock allow_tabs waits for resource ids", "the step: \(String(describing: step))")
#expect(sent.requests.isEmpty, "allow_tabs was sent with no ids")

// The tree reports the ids (the next snapshot): the step runs now, once, for the dock's screen and pane.
store.apply(snapshot: Self.tree(identified: true))
// Bounded by the suite's time limit; a step that never runs fails the expectations below once 2000 hops passed.
for _ in 0..<2000 where sent.requests.isEmpty { await Task.yield() }
#expect(sent.requests.map(\.screen) == [Self.screenResource], "allow_tabs did not run once the ids came: \(sent.requests)")
#expect(sent.requests.map(\.pane) == [Self.paneResource])
#expect(defaults.bool(forKey: HomeDock.dockTakesTabsKey), "the dock was not recorded as taking tabs")
}
}
Loading