Make "Always" confirmation behavior setting work by handling it for ongoing sync patches

- Add "Always" option to confirmationBehavior setting
- Route WebSocket patches through the confirmation callback
- Fix UI not transitioning back to Connected after confirming ongoing patches
- Fix DomLabel Flipper motor cleanup on unmount to prevent errors

Author: warsam

plugin/src/App/Components/PatchVisualizer/DomLabel.lua

@@ -72,6 +72,11 @@ function DomLabel:init()
 	end)
 end
 
+function DomLabel:willUnmount()
+	-- Stop the motor to prevent onStep callbacks after unmount
+	self.motor:stop()
+end
+
 function DomLabel:didUpdate(prevProps)
 	if
 		prevProps.instance ~= self.props.instance

plugin/src/App/init.lua

@@ -751,36 +751,38 @@ function App:startSession()
 		end
 
 		local confirmationBehavior = Settings:get("confirmationBehavior")
-		if confirmationBehavior == "Initial" then
-			-- Only confirm if we haven't synced this project yet this session
-			if self.knownProjects[serverInfo.projectName] then
-				Log.trace(
-					"Accepting patch without confirmation because project has already been connected and behavior is set to Initial"
-				)
-				return "Accept"
-			end
-		elseif confirmationBehavior == "Large Changes" then
-			-- Only confirm if the patch impacts many instances
-			if PatchSet.countInstances(patch) < Settings:get("largeChangesConfirmationThreshold") then
-				Log.trace(
-					"Accepting patch without confirmation because patch is small and behavior is set to Large Changes"
-				)
-				return "Accept"
-			end
-		elseif confirmationBehavior == "Unlisted PlaceId" then
-			-- Only confirm if the current placeId is not in the servePlaceIds allowlist
-			if serverInfo.expectedPlaceIds then
-				local isListed = table.find(serverInfo.expectedPlaceIds, game.PlaceId) ~= nil
-				if isListed then
+		if confirmationBehavior ~= "Always" then
+			if confirmationBehavior == "Initial" then
+				-- Only confirm if we haven't synced this project yet this session
+				if self.knownProjects[serverInfo.projectName] then
 					Log.trace(
-						"Accepting patch without confirmation because placeId is listed and behavior is set to Unlisted PlaceId"
+						"Accepting patch without confirmation because project has already been connected and behavior is set to Initial"
 					)
 					return "Accept"
 				end
+			elseif confirmationBehavior == "Large Changes" then
+				-- Only confirm if the patch impacts many instances
+				if PatchSet.countInstances(patch) < Settings:get("largeChangesConfirmationThreshold") then
+					Log.trace(
+						"Accepting patch without confirmation because patch is small and behavior is set to Large Changes"
+					)
+					return "Accept"
+				end
+			elseif confirmationBehavior == "Unlisted PlaceId" then
+				-- Only confirm if the current placeId is not in the servePlaceIds allowlist
+				if serverInfo.expectedPlaceIds then
+					local isListed = table.find(serverInfo.expectedPlaceIds, game.PlaceId) ~= nil
+					if isListed then
+						Log.trace(
+							"Accepting patch without confirmation because placeId is listed and behavior is set to Unlisted PlaceId"
+						)
+						return "Accept"
+					end
+				end
+			elseif confirmationBehavior == "Never" then
+				Log.trace("Accepting patch without confirmation because behavior is set to Never")
+				return "Accept"
 			end
-		elseif confirmationBehavior == "Never" then
-			Log.trace("Accepting patch without confirmation because behavior is set to Never")
-			return "Accept"
 		end
 
 		-- The datamodel name gets overwritten by Studio, making confirmation of it intrusive
@@ -821,7 +823,22 @@ function App:startSession()
 			timeout = 7,
 		})
 
-		return self.confirmationEvent:Wait()
+		local result = self.confirmationEvent:Wait()
+
+		-- Reset UI state back to Connected after confirmation
+		-- This is needed for ongoing WebSocket patches where the session
+		-- is already connected and won't trigger a status change
+		if self.serveSession and self.serveSession:getStatus() == ServeSession.Status.Connected then
+			self:setState({
+				appStatus = AppStatus.Connected,
+				toolbarIcon = Assets.Images.PluginButtonConnected,
+				-- Clear patchTree to avoid animation issues when the
+				-- PatchVisualizer unmounts while Flipper motors are running
+				patchTree = nil,
+			})
+		end
+
+		return result
 	end)
 
 	serveSession:start()

plugin/src/ServeSession.lua

@@ -104,6 +104,8 @@ function ServeSession.new(options)
 		__instanceMap = instanceMap,
 		__changeBatcher = changeBatcher,
 		__statusChangedCallback = nil,
+		__userConfirmCallback = nil,
+		__serverInfo = nil,
 		__connections = connections,
 		__precommitCallbacks = {},
 		__postcommitCallbacks = {},
@@ -196,6 +198,7 @@ function ServeSession:start()
 		:connect()
 		:andThen(function(serverInfo)
 			self:setLoadingText("Loading initial data from server...")
+			self.__serverInfo = serverInfo
 			return self:__initialSync(serverInfo):andThen(function()
 				self:setLoadingText("Starting sync loop...")
 				self:__setStatus(Status.Connected, serverInfo.projectName)
@@ -209,9 +212,33 @@ function ServeSession:start()
 
 						Log.debug("Received {} messages from Rojo server", #messagesPacket.messages)
 
+						-- Combine all messages into a single patch
+						local combinedPatch = PatchSet.newEmpty()
 						for _, message in messagesPacket.messages do
-							self:__applyPatch(message)
+							PatchSet.assign(combinedPatch, message)
 						end
+
+						-- Spawn a new thread to handle potentially yielding confirmation
+						task.spawn(function()
+							Log.trace("Processing WebSocket patch, callback exists: {}", self.__userConfirmCallback ~= nil)
+							Log.trace("ServerInfo exists: {}", self.__serverInfo ~= nil)
+
+							local userDecision = "Accept"
+							if self.__userConfirmCallback ~= nil then
+								userDecision = self.__userConfirmCallback(self.__instanceMap, combinedPatch, self.__serverInfo)
+							end
+
+							Log.trace("WebSocket patch decision: {}", userDecision)
+
+							if userDecision == "Abort" then
+								self:__stopInternal("Aborted Rojo sync operation")
+								return
+							elseif userDecision == "Accept" then
+								self:__applyPatch(combinedPatch)
+							end
+							-- Note: "Reject" is not handled for ongoing patches, only for initial sync
+						end)
+
 						self.__apiContext:setMessageCursor(messagesPacket.messageCursor)
 					end,
 				})

plugin/src/Settings.lua

@@ -20,7 +20,7 @@ local defaultSettings = {
 	checkForUpdates = true,
 	checkForPrereleases = false,
 	autoConnectPlaytestServer = false,
-	confirmationBehavior = "Initial" :: "Never" | "Initial" | "Large Changes" | "Unlisted PlaceId",
+	confirmationBehavior = "Initial" :: "Never" | "Initial" | "Always" | "Large Changes" | "Unlisted PlaceId",
 	largeChangesConfirmationThreshold = 5,
 	playSounds = true,
 	typecheckingEnabled = false,