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
70 changes: 70 additions & 0 deletions Sources/Surfaces/CmuxTuiSurfaceProviders.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider {
private var stateRecoveryRefreshTask: Task<Void, Never>?
private var stateRecoveryRefreshQueued = false
private var stateRecoveryCount = 0
/// The cursor of the last full snapshot that disagreed with the installed
/// graph at the same cursor. The first conflict schedules a recovery read;
/// a second full snapshot conflicting at this cursor is adopted.
private(set) var equalCursorConflict: CloudVMCursor?
/// Set only by the install that armed ``equalCursorConflict``, so a read
/// that cannot adopt (stale, or fenced by a pending rename) at an already
/// armed cursor does not spend another recovery read.
private(set) var equalCursorConflictArmedByLastInstall = false
private static let stateRecoveryLimit = 5
private var changeWatcher: Task<Void, Never>?
/// Identity of the link owned by `changeWatcher`. A provider can replace a
Expand Down Expand Up @@ -182,6 +190,8 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider {
}
func suspendForFeatureFlag() {
isFeatureSuspended = true
// The first read after resuming must arm afresh, never adopt at once.
equalCursorConflict = nil
displayCoordinator.stop()
guestURLService?.stop()
guestURLService = nil
Expand Down Expand Up @@ -323,6 +333,12 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider {
let incoming = CmuxTuiSnapshotParser.state(fromSnapshot: object, machine: machine)
else { throw ProviderError.invalidSnapshot(machineID) }
let installed = installSnapshotIfNewer(incoming, requestVersion: requestVersion)
// A first equal-cursor conflict keeps the graph; read again so a
// repeated conflict can adopt the daemon's answer. When the budget
// is spent the conflict stays armed, and the next refresh adopts.
if !installed, equalCursorConflictArmedByLastInstall {
scheduleStateRecoveryRefresh()
}
// Equal cursors are a valid no-op refresh only when the revisioned
// graph is equivalent. A cursor alone is not proof
// that a malformed or misconfigured daemon returned the same graph.
Expand Down Expand Up @@ -442,8 +458,53 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider {
}
return snapshotEstablishedCurrentGraph
}
/// A full snapshot read at the installed cursor disagrees with the graph the
/// deltas built. One of them is wrong, and nothing later at this cursor
/// can reconcile them, so refusing forever would wedge the machine.
///
/// The first conflict keeps the installed graph and arms a recovery read
/// (the caller schedules it), so a single race cannot discard state. A
/// second full snapshot conflicting at the same cursor wins: a full read
/// at the current cursor is the daemon's own answer. Adoption replaces the
/// whole graph, exactly like any fresh install, so a field or key the
/// daemon does not send is absent afterwards rather than kept from the
/// delta-built graph. App-side overlays (pending renames, pending
/// creations) live outside `cloudState` and are reapplied on publish.
/// Event-feed snapshots and reads that started before a newer install
/// never count: only a current full refresh can arm or adopt.
private func resolveEqualCursorConflict(incoming: CloudVMState, requestVersion: UInt64?) -> Bool {
guard let requestVersion, requestVersion == cloudStateInstallVersion, let cursor = incoming.cursor else {
#if DEBUG
cmuxDebugLog("cloud.state.snapshotIgnored machine=\(machineID) reason=equal-cursor-conflict")
#endif
return false
}
guard equalCursorConflict == cursor else {
equalCursorConflict = cursor
equalCursorConflictArmedByLastInstall = true
#if DEBUG
cmuxDebugLog("cloud.state.snapshotIgnored machine=\(machineID) reason=equal-cursor-conflict armed=1")
#endif
return false
}
cloudState = incoming
cloudStateInstallVersion &+= 1
equalCursorConflict = nil
// The recovery read that armed this succeeded; it must not count
// against the budget later event-feed barriers rely on.
stateRecoveryCount = 0
retirePendingRemoteRenames(observed: incoming)
sentryBreadcrumb(
"cloud.state.equalCursorConflictAdopted",
category: "cloud",
data: ["machine": machineID, "revision": String(cursor.revision)]
)
return true
}

@discardableResult
func installSnapshotIfNewer(_ incoming: CloudVMState, requestVersion: UInt64? = nil) -> Bool {
equalCursorConflictArmedByLastInstall = false
guard acceptsIncomingGeneration(incoming.cursor) else {
#if DEBUG
cmuxDebugLog("cloud.state.snapshotIgnored machine=\(machineID) reason=old-generation")
Expand All @@ -455,6 +516,11 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider {
// rename: a delayed equal-cursor predecessor must not look current.
if let current = cloudState, current.cursor == incoming.cursor {
guard current.hasSameRevisionedContent(as: incoming), incomingPassesPendingRenameFence(incoming) else {
// A pending rename's predecessor is refused outright; only a
// content conflict can arm recovery.
if incomingPassesPendingRenameFence(incoming), !current.hasSameRevisionedContent(as: incoming) {
return resolveEqualCursorConflict(incoming: incoming, requestVersion: requestVersion)
}
#if DEBUG
cmuxDebugLog("cloud.state.snapshotIgnored machine=\(machineID) reason=equal-cursor-conflict")
#endif
Expand All @@ -465,6 +531,7 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider {
}
cloudState = incoming
cloudStateInstallVersion &+= 1
equalCursorConflict = nil
retirePendingRemoteRenames(observed: incoming)
return true
}
Expand All @@ -491,6 +558,7 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider {
}
cloudState = incoming
cloudStateInstallVersion &+= 1
equalCursorConflict = nil
if let generation = incoming.cursor?.generation {
acceptedCloudGenerations.insert(generation)
}
Expand Down Expand Up @@ -1454,6 +1522,8 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider {
retirePendingRemoteRenames(observed: next)
eventsFeedWarning = nil
clearStateRecovery()
// The cursor moved on, so an armed conflict no longer applies.
equalCursorConflict = nil
await link.setEventsCursor(next.cursor)
guard watchedLink === link, canPublishCloudState(next) else { return }
info.linkState = .connected
Expand Down
4 changes: 4 additions & 0 deletions cmux.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,7 @@
F3FAB983EE90414CAA90F970 /* CloudDirectoryTestFixture.swift in Sources */ = {isa = PBXBuildFile; fileRef = FFC7BA3C7C834D20B3173D36 /* CloudDirectoryTestFixture.swift */; };
33E9D640ED2646A38673776D /* CloudDisplayCatalogTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B7DF8AE123341DEBA066731 /* CloudDisplayCatalogTests.swift */; };
C10D0A200000000000000001 /* CloudDomainsCLIIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C10D0A200000000000000002 /* CloudDomainsCLIIntegrationTests.swift */; };
71FE9F238B1717D43FC01E63 /* CloudEqualCursorConflictRecoveryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D4316FCA6BA8B1260D8FFB6 /* CloudEqualCursorConflictRecoveryTests.swift */; };
5EDE6BFD28BE4BD58EF84889 /* CloudFeatureAvailabilityObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 668F7B9BC77445B793C0298A /* CloudFeatureAvailabilityObserver.swift */; };
C3CD2FF9F6F1489C8EDD0C7B /* CloudFeatureFlagTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EBF28FA7AB264C1AA7821248 /* CloudFeatureFlagTests.swift */; };
CFSF00000000000000000011 /* CloudFileExplorerBehaviorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CFSR00000000000000000011 /* CloudFileExplorerBehaviorTests.swift */; };
Expand Down Expand Up @@ -4882,6 +4883,7 @@
FFC7BA3C7C834D20B3173D36 /* CloudDirectoryTestFixture.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = CloudDirectoryTestFixture.swift; sourceTree = "<group>"; };
6B7DF8AE123341DEBA066731 /* CloudDisplayCatalogTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CloudDisplayCatalogTests.swift"; sourceTree = "<group>"; };
C10D0A200000000000000002 /* CloudDomainsCLIIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloudDomainsCLIIntegrationTests.swift; sourceTree = "<group>"; };
2D4316FCA6BA8B1260D8FFB6 /* CloudEqualCursorConflictRecoveryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CloudEqualCursorConflictRecoveryTests.swift"; sourceTree = "<group>"; };
668F7B9BC77445B793C0298A /* CloudFeatureAvailabilityObserver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CloudFeatureAvailabilityObserver.swift"; sourceTree = "<group>"; };
EBF28FA7AB264C1AA7821248 /* CloudFeatureFlagTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloudFeatureFlagTests.swift; sourceTree = "<group>"; };
CFSR00000000000000000011 /* CloudFileExplorerBehaviorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloudFileExplorerBehaviorTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -12339,6 +12341,7 @@
AE748DECE9679B649874A1FE /* WorkspaceActivityReorderControllerTests.swift */,
751E2E409D9D8EED12988A80 /* SplitSpaceTests.swift */,
D0C6C5413708B2C95D6C1CC3 /* NarrowWindowSidePanelFitTests.swift */,
2D4316FCA6BA8B1260D8FFB6 /* CloudEqualCursorConflictRecoveryTests.swift */,
);
path = cmuxTests;
sourceTree = "<group>";
Expand Down Expand Up @@ -16119,6 +16122,7 @@
F3FAB983EE90414CAA90F970 /* CloudDirectoryTestFixture.swift in Sources */,
33E9D640ED2646A38673776D /* CloudDisplayCatalogTests.swift in Sources */,
C10D0A200000000000000001 /* CloudDomainsCLIIntegrationTests.swift in Sources */,
71FE9F238B1717D43FC01E63 /* CloudEqualCursorConflictRecoveryTests.swift in Sources */,
C3CD2FF9F6F1489C8EDD0C7B /* CloudFeatureFlagTests.swift in Sources */,
CFSF00000000000000000011 /* CloudFileExplorerBehaviorTests.swift in Sources */,
0A42EBEFBFD8771DC2D5C061 /* CloudFileRootOwnershipTests.swift in Sources */,
Expand Down
147 changes: 147 additions & 0 deletions cmuxTests/CloudEqualCursorConflictRecoveryTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import CmuxCloud
import CmuxSurfaceCatalogModel
import Foundation
import Testing

#if canImport(cmux_DEV)
@testable import cmux_DEV
#elseif canImport(cmux)
@testable import cmux
#endif

/// A full snapshot and the applied deltas can disagree at the same cursor. The
/// first such conflict from a full refresh arms a recovery read; a second one
/// at the same cursor adopts the fresh snapshot, so the graph never stays
/// wedged, while a single race cannot throw away the installed graph.
@MainActor
@Suite("Equal-cursor conflict recovery", .timeLimit(.minutes(1)))
struct CloudEqualCursorConflictRecoveryTests {
/// The provider holds its catalog `unowned`, so the suite owns it for the
/// whole test; a temporary would be freed before the provider touches it.
private let catalog = SurfaceCatalog()

private func snapshot(workspaceName: String, revision: Int = 3) -> [String: Any] {
[
"cursor": ["generation": "daemon", "revision": String(revision)],
"workspaces": [["id": "ws_main", "name": workspaceName, "focused": true]],
"screens": [["id": "screen", "workspace_id": "ws_main"]],
"panes": [["id": "pane", "screen_id": "screen"]],
"tabs": [["id": "tab", "pane_id": "pane", "content_kind": "terminal", "content_id": "term", "focused": true]],
"terminals": [["id": "term", "title": "bash", "lifecycle": "running"]],
"browsers": [], "agents": []
]
}

private func makeProvider() -> CmuxTuiSurfaceProvider {
let summary = VMSummary(id: "equal-cursor", provider: "freestyle", status: "running", image: "cmux-devbox", createdAt: 0, base: nil)
return CmuxTuiSurfaceProvider(
summary: summary,
links: CloudMachineLinkManager(clientURL: nil, hostThemeColors: { nil }),
catalog: catalog
)
}

private func state(_ provider: CmuxTuiSurfaceProvider, _ name: String, revision: Int = 3) throws -> CloudVMState {
try #require(CmuxTuiSnapshotParser.state(fromSnapshot: snapshot(workspaceName: name, revision: revision), machine: provider.machine))
}

private func name(_ provider: CmuxTuiSurfaceProvider) -> String? {
provider.cloudState?.lookupIndex.workspace(id: "ws_main")?.name
}

@Test("The first full-refresh conflict keeps the graph and arms recovery; a second adopts the fresh snapshot")
func secondConflictAdopts() throws {
let provider = makeProvider()
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied")))

let fresh = try state(provider, "Daemon")
#expect(!provider.installSnapshotIfNewer(fresh, requestVersion: 1))
#expect(name(provider) == "Applied")
#expect(provider.equalCursorConflict == fresh.cursor)

#expect(provider.installSnapshotIfNewer(fresh, requestVersion: 1), "a repeated conflict at the same cursor must break the wedge")
#expect(name(provider) == "Daemon")
#expect(provider.equalCursorConflict == nil)
}

@Test("An event-feed snapshot conflict never adopts")
func eventConflictNeverAdopts() throws {
let provider = makeProvider()
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied")))
let fresh = try state(provider, "Daemon")
#expect(!provider.installSnapshotIfNewer(fresh))
#expect(!provider.installSnapshotIfNewer(fresh))
#expect(name(provider) == "Applied")
}

@Test("A read that started before a newer install never adopts")
func staleReadNeverAdopts() throws {
let provider = makeProvider()
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied")))
let fresh = try state(provider, "Daemon")
#expect(!provider.installSnapshotIfNewer(fresh, requestVersion: 0))
#expect(!provider.installSnapshotIfNewer(fresh, requestVersion: 0))
#expect(name(provider) == "Applied")
}

@Test("Only the arming install asks for a recovery read")
func onlyArmingSchedulesRecovery() throws {
let provider = makeProvider()
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied")))
let fresh = try state(provider, "Daemon")
#expect(!provider.installSnapshotIfNewer(fresh, requestVersion: 1))
#expect(provider.equalCursorConflictArmedByLastInstall)
// A stale read at the armed cursor cannot adopt, so it must not spend
// another recovery read either.
#expect(!provider.installSnapshotIfNewer(fresh, requestVersion: 0))
#expect(!provider.equalCursorConflictArmedByLastInstall)
#expect(provider.equalCursorConflict == fresh.cursor)
}

@Test("A pending rename's predecessor is never adopted, however often it conflicts")
func renameFenceHoldsWhileArmed() throws {
let provider = makeProvider()
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied")))
let fresh = try state(provider, "Daemon")
provider.recordPendingRemoteRename(tabID: "tab", name: "Renamed", receipt: try #require(fresh.cursor))
#expect(!provider.installSnapshotIfNewer(fresh, requestVersion: 1))
#expect(!provider.installSnapshotIfNewer(fresh, requestVersion: 1))
#expect(name(provider) == "Applied")
#expect(provider.equalCursorConflict == nil)
}

@Test("Suspending clears an armed conflict so the first read after resume cannot adopt")
func suspendClearsConflict() throws {
let provider = makeProvider()
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied")))
#expect(!provider.installSnapshotIfNewer(try state(provider, "Daemon"), requestVersion: 1))
#expect(provider.equalCursorConflict != nil)
provider.suspendForFeatureFlag()
#expect(provider.equalCursorConflict == nil)
}

@Test("An install at a newer cursor clears an armed conflict")
func newerInstallClearsConflict() throws {
let provider = makeProvider()
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied")))
#expect(!provider.installSnapshotIfNewer(try state(provider, "Daemon"), requestVersion: 1))
#expect(provider.equalCursorConflict != nil)
#expect(provider.installSnapshotIfNewer(try state(provider, "Next", revision: 4)))
#expect(provider.equalCursorConflict == nil)

// A conflict at the new cursor starts over: the first one keeps the graph.
#expect(!provider.installSnapshotIfNewer(try state(provider, "Other", revision: 4), requestVersion: 2))
#expect(name(provider) == "Next")
}

@Test("An equal-content install at the armed cursor clears the conflict")
func equalContentInstallClearsConflict() throws {
let provider = makeProvider()
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied")))
#expect(!provider.installSnapshotIfNewer(try state(provider, "Daemon"), requestVersion: 1))
#expect(provider.equalCursorConflict != nil)
#expect(provider.installSnapshotIfNewer(try state(provider, "Applied"), requestVersion: 1))
#expect(provider.equalCursorConflict == nil)
#expect(name(provider) == "Applied")
}
}