diff --git a/AI-canary.txt b/AI-canary.txt new file mode 100644 index 00000000..642d9905 --- /dev/null +++ b/AI-canary.txt @@ -0,0 +1 @@ +Used AI to create init.spec.luau to test the code \ No newline at end of file diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index e6e4deee..e1d602bb 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -601,6 +601,13 @@ function App:useRunningConnectionInfo() end function App:startSession() + if self.serveSession ~= nil then + -- A session is already active + -- Without this guard double clicking connect starts 2 serve sessions and leaks the first and crashes the plugn + Log.trace("Ignoring startSession because a session is already active") + return + end + local claimedLock, priorOwner = self:claimSyncLock() if not claimedLock then local msg = string.format("Could not sync because user '%s' is already syncing", tostring(priorOwner)) @@ -630,6 +637,8 @@ function App:startSession() apiContext = apiContext, twoWaySync = Settings:get("twoWaySync"), }) + -- Assign before start() so the guard above rejects simultaneous calls + self.serveSession = serveSession serveSession:setUpdateLoadingTextCallback(function(text: string) self:setState({ @@ -825,8 +834,6 @@ function App:startSession() end) serveSession:start() - - self.serveSession = serveSession end function App:endSession() diff --git a/plugin/src/App/init.spec.lua b/plugin/src/App/init.spec.lua new file mode 100644 index 00000000..50d65cf9 --- /dev/null +++ b/plugin/src/App/init.spec.lua @@ -0,0 +1,101 @@ +return function() + local App = require(script.Parent) + local ServeSession = require(script.Parent.Parent.ServeSession) + local ApiContext = require(script.Parent.Parent.ApiContext) + + -- The module returns a wrapper function that renders the App component. + -- The App class itself isn't exported, so we pull it off the element it + -- creates in order to call its methods directly. + local AppComponent = App({}).component + + -- A stubbed ServeSession that records nothing and does nothing, so that + -- startSession can run without touching the network. + local function makeServeSessionStub() + return { + setUpdateLoadingTextCallback = function() end, + hookPrecommit = function() + return function() end + end, + hookPostcommit = function() + return function() end + end, + onStatusChanged = function() end, + setConfirmCallback = function() end, + start = function() end, + } + end + + -- A minimal `self` with just the methods startSession touches, so we can + -- invoke App.startSession without mounting the whole component. + local function makeFakeSelf() + return { + serveSession = nil, + knownProjects = {}, + -- claimSyncLock returns true in solo (no Team Create), which is the + -- "quick session" case the double-click bug happens in. + claimSyncLock = function() + return true + end, + getHostAndPort = function() + return "localhost", "34872" + end, + isAutoConnectPlaytestServerAvailable = function() + return false + end, + setState = function() end, + addNotification = function() end, + setPriorSyncInfo = function() end, + setRunningConnectionInfo = function() end, + clearRunningConnectionInfo = function() end, + releaseSyncLock = function() end, + } + end + + describe("startSession", function() + local realServeSessionNew = ServeSession.new + local realApiContextNew = ApiContext.new + local created + + beforeEach(function() + created = 0 + ServeSession.new = function() + created += 1 + return makeServeSessionStub() + end + -- Stub ApiContext so construction stays offline and deterministic. + ApiContext.new = function() + return {} + end + end) + + afterEach(function() + ServeSession.new = realServeSessionNew + ApiContext.new = realApiContextNew + end) + + it("creates a session on the first call", function() + local self = makeFakeSelf() + AppComponent.startSession(self) + expect(created).to.equal(1) + end) + + it("does not start a second session when called again while active", function() + local self = makeFakeSelf() + -- Two calls in a row mimic a double-click firing startSession twice + -- before the first session finishes. Without the re-entrancy guard + -- this creates two ServeSessions and leaks the first. + AppComponent.startSession(self) + AppComponent.startSession(self) + expect(created).to.equal(1) + end) + + it("allows a new session after the previous one is cleared", function() + local self = makeFakeSelf() + AppComponent.startSession(self) + -- Disconnecting clears serveSession; a later connect should work. + self.serveSession = nil + AppComponent.startSession(self) + expect(created).to.equal(2) + end) + end) +end