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
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ final class PaneController: SurfacePresenter, PresentablePane {

// MARK: Lookup

func tab(_ id: StripTabID) -> TabModel? { pane.tabs.first { $0.id == id.rawValue } }
func tab(_ id: StripTabID) -> TabModel? { pane.tabs.first { $0.id == id.rawValue } ?? daemon.store.creations.replacement(ofStandIn: id.rawValue, in: pane) }

var selectedTab: TabModel? { stripModel.selectedID.flatMap(tab) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ import Foundation
/// only if a snapshot says the daemon kept it).
public static let closeAttemptLimit = 3

/// The tab of `pane` that stand-in tab `id` became, once the records list it there in its place
/// (a confirmed reply or echo named it). A pane's strip shows the records one observation step
/// late: a Cmd-W in that step still names the stand-in, which the records no longer hold, and
/// closed nothing (smoke, STATUS 4852). Nil for any other id, and for a stand-in still shown,
/// cancelled (its tab is hidden) or refused.
public func replacement(ofStandIn id: String, in pane: PaneModel) -> TabModel? {
guard ProvisionalTab.isProvisional(id), let surface = store.intentLog.replacement(ofStandIn: id) else { return nil }
return pane.tabs.first { $0.surface == surface }
}

/// Whether tab `id` is still a pending creation's provisional tab (a close cancels it).
public func isPending(tab id: String) -> Bool { CancelledCreation.transaction(showingTab: id, in: store) != nil }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ struct IntentLog {
/// sequence from the previous one no longer compares: an intent whose
/// reply came settles with the new connection's first snapshot.
mutating func connectionReplaced() {
replacedStandIns.removeAll() // another daemon may reuse their surfaces
entries = entries.map { entry in
var entry = entry
if entry.settleSequence != nil {
Expand Down Expand Up @@ -214,6 +215,15 @@ struct IntentLog {

func wasCancelled(_ transaction: ClientTransactionID) -> Bool { cancelled.contains(transaction) }

/// The daemon surface each recently created stand-in tab became, by the stand-in's tab id
/// (bounded), still known after its intent left the log: a pane's strip shows the records one
/// observation step late, and a key acting on the stand-in it still selects acts on that tab
/// (``DaemonCreations/replacement(ofStandIn:in:)``).
private var replacedStandIns: [(standIn: String, surface: SurfaceID)] = []
private static let replacedStandInLimit = 64

func replacement(ofStandIn id: String) -> SurfaceID? { replacedStandIns.last { $0.standIn == id }?.surface }

/// False when `transaction` is not pending.
mutating func markCloseWhenCreated(_ transaction: ClientTransactionID) -> Bool {
guard entries.modifyFirst(where: { $0.transaction == transaction }, { $0.closeWhenCreated = true }) else { return false }
Expand Down Expand Up @@ -255,6 +265,12 @@ struct IntentLog {
if confirmed { $0.createdConfirmed = true }
}
guard confirmed else { return }
if case .createTab(_, let provisional)? = entries.first(where: { $0.transaction == transaction })?.kind,
let standIn = provisional.tabResourceID?.rawValue {
replacedStandIns.removeAll { $0.standIn == standIn }
replacedStandIns.append((standIn, surface))
if replacedStandIns.count > Self.replacedStandInLimit { replacedStandIns.removeFirst(replacedStandIns.count - Self.replacedStandInLimit) }
}
entries = entries.map { entry in
guard entry.transaction != transaction, entry.createdSurface == surface, !entry.createdConfirmed else { return entry }
var entry = entry
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import AppKit
import CmuxNextActions
@testable import CmuxNextApp
@testable import CmuxNextDaemon
import CmuxNextTabs
import Foundation
import Testing

/// Tip smoke red at b732874bf5e3 (STATUS 4852): Cmd-T on a browser page, then Cmd-W. The New Tab
/// page stayed and the browser page never came back. Cmd-T shows the page as a stand-in tab
/// (`pending_tab_...`) until the daemon's reply names the store's tab, and the store then swaps
/// them in one step. The pane's strip shows the store one observation step later, so a Cmd-W in
/// that step still named the stand-in, which the store no longer listed: Close Tab found no tab
/// and closed nothing, silently. On the fleet (hog +15 GB) a Cmd-W 30 ms after Cmd-T hit it in 3
/// of 3 rounds, on the tip and on the last smoke-green build. Runs the app's Close Tab against a
/// same-tree cmux-tui daemon, in the turn the store swaps the tabs, before any observation step.
@MainActor @Suite(.serialized, .timeLimit(.minutes(3)), .enabled(if: SameTreeDaemonBinary.url != nil, "needs the same-tree cmux-tui"))
struct CmdWOnReplacedStandInTests {
@Test func cmdWInTheStepTheStoreReplacedTheNewTabStandInClosesTheStoresTab() async throws {
try await PinnedPageTests.withApp { services, connection in
let store = services.daemon.store
let pane = try #require(services.windows.active?.focusedPane)
let handle = pane.pane.handle

// The browser page Cmd-T is pressed on, selected.
let page = try await connection.newFrontendBrowserTab(url: "https://example.com/", engine: .webkit, in: handle, activate: false)
try await PinnedPageTests.until({ store.tab(surface: page.surface).map { pane.orderedIDs.map(\.rawValue).contains($0.id) } == true })
let pageID = try #require(store.tab(surface: page.surface)).id
pane.select(TabID(pageID))

// The tab the daemon makes for Cmd-T's New Tab page, listed (its tab-added came first).
let record = AgentSessionRef(host: AgentSessionRef.host(installID: "cn-cmdw-stand-in"), hostName: "This Mac",
session: nil, harness: nil)
let made = try await connection.request(NewConversationTabRequest(agentSession: record, pane: handle))
try await PinnedPageTests.until({ store.tab(surface: made.surface).map { pane.orderedIDs.map(\.rawValue).contains($0.id) } == true })
let madeID = try #require(store.tab(surface: made.surface)).id

// Cmd-T (AgentTabStore.open): the New Tab page shows as a stand-in, selected.
let standIn = ProvisionalTab()
let transaction = ClientTransactionID.generate()
store.intend(.createTab(pane: handle, provisional: standIn.conversation(ConversationTabRef(agentSession: record))),
transaction: transaction)
pane.selectWhenReported(surface: standIn.surface)
try #require(pane.stripModel.selectedID?.rawValue == standIn.id, "the stand-in is not selected")

// The reply's turn, as AgentTabStore.open runs it: the selection waits for the store's tab
// (moveSelection), then the reply names it and it replaces the stand-in in the records.
pane.pendingSelectSurface = made.surface
ProvisionalTab.created(transaction, surface: made.surface, in: store)
store.noteSettledAtNextSnapshot(transaction)
#expect(store.tab(id: standIn.id) == nil, "the store still lists the stand-in")
// The strip has not caught up yet (its observation runs in a later turn): Cmd-W now.
try #require(pane.stripModel.selectedID?.rawValue == standIn.id,
"the strip already selects \(pane.stripModel.selectedID?.rawValue ?? "-"): the test no longer reaches the step")
#expect(services.registry.perform("closeTab", invocation: ActionInvocation(origin: .user)))

try await PinnedPageTests.until({ store.tab(surface: made.surface) == nil },
within: .seconds(5),
report: { "Cmd-W closed nothing: the pane holds \(store.pane(handle)?.tabs.map(\.id) ?? []), the New Tab tab is \(madeID)" })
#expect(store.tab(surface: page.surface) != nil, "Cmd-W closed the browser page")
}
}
}
Loading