From 69538990453ce49a15a2c21b992179d130214efb Mon Sep 17 00:00:00 2001
From: kisfenyo <nagyfenyvesi.viktor@gmail.com>
Date: Sat, 13 Jun 2026 22:58:59 +0200
Subject: [PATCH] fix(M25): atomic.Pointer for Server.integrationMgr
 (constructor-goroutine race)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

NewServer launches the SyncFileBrowserMounts goroutine (reads integrationMgr)
from the constructor, BEFORE main.go's SetIntegrationManager write — so the
init-only happens-before that covers the other Set* fields does NOT hold here,
making it a genuine data race (handlers.go:358/360/1433 reads vs server.go:162
write). Converted the field to atomic.Pointer[integrations.Manager]; setter
Stores, all 3 readers Load(). Regression test reproduces the concurrent access
(clean under -race; flags on the pre-fix plain-pointer field).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 controller/internal/web/handlers.go           |  8 +--
 .../internal/web/integrationmgr_race_test.go  | 54 +++++++++++++++++++
 controller/internal/web/server.go             | 11 ++--
 3 files changed, 66 insertions(+), 7 deletions(-)
 create mode 100644 controller/internal/web/integrationmgr_race_test.go

diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go
index 80603bb..2219e58 100644
--- a/controller/internal/web/handlers.go
+++ b/controller/internal/web/handlers.go
@@ -355,9 +355,9 @@ func (s *Server) deployHandler(w http.ResponseWriter, r *http.Request, name stri
 	// renders those sections.
 	if alreadyDeployed {
 		// App-to-app integrations
-		if meta.HasIntegrations() && s.integrationMgr != nil {
+		if im := s.integrationMgr.Load(); meta.HasIntegrations() && im != nil {
 			data["HasIntegrations"] = true
-			data["Integrations"] = s.integrationMgr.ListForProvider(meta.Slug)
+			data["Integrations"] = im.ListForProvider(meta.Slug)
 		}
 
 		// Geo-restriction per-app data
@@ -1430,8 +1430,8 @@ func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) {
 	}
 
 	// Re-apply active integrations into config.yaml (before container restart)
-	if s.integrationMgr != nil {
-		s.integrationMgr.ReapplyConfigForTarget("filebrowser")
+	if im := s.integrationMgr.Load(); im != nil {
+		im.ReapplyConfigForTarget("filebrowser")
 	}
 
 	// Generate and write compose (includes config.yaml mount)
diff --git a/controller/internal/web/integrationmgr_race_test.go b/controller/internal/web/integrationmgr_race_test.go
new file mode 100644
index 0000000..88a3e7c
--- /dev/null
+++ b/controller/internal/web/integrationmgr_race_test.go
@@ -0,0 +1,54 @@
+package web
+
+import (
+	"sync"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/integrations"
+)
+
+// TestIntegrationManagerNoRace is a regression test for M25. NewServer launches
+// the SyncFileBrowserMounts goroutine (which reads s.integrationMgr) from inside
+// the constructor, BEFORE main.go calls SetIntegrationManager. With a plain
+// pointer field that write/read pair is a data race; this test reproduces the
+// concurrent access and must run clean under `go test -race`.
+//
+// On the pre-fix code (plain `integrationMgr *integrations.Manager`, written via
+// `s.integrationMgr = mgr` and read via `s.integrationMgr`) the race detector
+// flags this. With the atomic.Pointer field it is clean. Run with:
+//
+//	go test -race ./internal/web/ -run IntegrationManagerNoRace
+func TestIntegrationManagerNoRace(t *testing.T) {
+	var s Server // zero value: atomic.Pointer field is usable as-is
+
+	const iters = 2000
+	var wg sync.WaitGroup
+
+	// Writer — mirrors main.go calling SetIntegrationManager after construction.
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for i := 0; i < iters; i++ {
+			s.SetIntegrationManager(&integrations.Manager{})
+		}
+	}()
+
+	// Reader — mirrors the constructor-launched SyncFileBrowserMounts goroutine
+	// reading the field concurrently with the write above.
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for i := 0; i < iters; i++ {
+			_ = s.integrationMgr.Load()
+		}
+	}()
+
+	wg.Wait()
+
+	// And the value round-trips.
+	m := &integrations.Manager{}
+	s.SetIntegrationManager(m)
+	if s.integrationMgr.Load() != m {
+		t.Fatal("SetIntegrationManager/Load round-trip mismatch")
+	}
+}
diff --git a/controller/internal/web/server.go b/controller/internal/web/server.go
index 3ee91a3..a39dd7a 100644
--- a/controller/internal/web/server.go
+++ b/controller/internal/web/server.go
@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"gitea.dooplex.hu/admin/felhom-controller/internal/appexport"
@@ -56,8 +57,12 @@ type Server struct {
 	// Asset syncer for Hub-managed assets (optional)
 	assetsSyncer *assets.Syncer
 
-	// App-to-app integration manager (optional)
-	integrationMgr *integrations.Manager
+	// App-to-app integration manager (optional).
+	// M25: atomic because the constructor launches the SyncFileBrowserMounts
+	// goroutine (which reads this) BEFORE SetIntegrationManager runs in main.go —
+	// so a plain field would be a data race (the init-only happens-before that
+	// covers the other Set* fields does NOT hold for this one).
+	integrationMgr atomic.Pointer[integrations.Manager]
 
 	// App export/import engine (optional)
 	appExporter *appexport.Exporter
@@ -159,7 +164,7 @@ func (s *Server) SetAssetsSyncer(as *assets.Syncer) {
 
 // SetIntegrationManager sets the app-to-app integration manager.
 func (s *Server) SetIntegrationManager(mgr *integrations.Manager) {
-	s.integrationMgr = mgr
+	s.integrationMgr.Store(mgr)
 }
 
 // SetLogBuffer sets the in-memory log ring buffer for the debug log viewer.