From 914e5c5a83eaeb8b46ea74be08d28fa7880787dc Mon Sep 17 00:00:00 2001 From: FiranclyWin Date: Sat, 25 Jul 2026 17:22:31 +0800 Subject: [PATCH 1/2] fix(plugin): guard startSession against double-click re-entry Closes #1046 (AI-assisted) --- AI-canary.txt | 1 + plugin/src/App/init.lua | 11 +++- plugin/src/App/init.spec.lua | 101 +++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 AI-canary.txt create mode 100644 plugin/src/App/init.spec.lua diff --git a/AI-canary.txt b/AI-canary.txt new file mode 100644 index 00000000..6f2352bb --- /dev/null +++ b/AI-canary.txt @@ -0,0 +1 @@ +This pull request is AI generated \ 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 From 60aee0cfb02148fb706076663bd59d3e74970da7 Mon Sep 17 00:00:00 2001 From: FiranclyWin Date: Sat, 25 Jul 2026 17:35:07 +0800 Subject: [PATCH 2/2] updated ai-canary --- AI-canary.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AI-canary.txt b/AI-canary.txt index 6f2352bb..642d9905 100644 --- a/AI-canary.txt +++ b/AI-canary.txt @@ -1 +1 @@ -This pull request is AI generated \ No newline at end of file +Used AI to create init.spec.luau to test the code \ No newline at end of file