PE-7412-saturn-reload-bug-fixing

.gitignore

@@ -32,3 +32,4 @@ Thumbs.db
 
 mcp-servers.json
 config.json
+settings.local.json

cmd/main.go

@@ -135,15 +135,15 @@ func main() {
 }
 
 // runMainApplication contains the core application logic that can be reloaded
-func runMainApplication(logger *logging.Logger) error {
+func runMainApplication(ctx context.Context, logger *logging.Logger) error {
 	// Load and prepare configuration
 	cfg := loadAndPrepareConfig(logger)
 
 	// Initialize MCP clients and discover tools
 	mcpClients, discoveredTools := initializeMCPClients(logger, cfg)
 
 	// Initialize and run Slack client
-	startSlackClient(logger, mcpClients, discoveredTools, cfg)
+	startSlackClient(ctx, logger, mcpClients, discoveredTools, cfg)
 
 	return nil
 }
@@ -564,7 +564,7 @@ func logLLMSettings(logger *logging.Logger, cfg *config.Config) {
 
 // startSlackClient starts the Slack client and handles shutdown
 // Use mcp.Client from the internal mcp package
-func startSlackClient(logger *logging.Logger, mcpClients map[string]*mcp.Client, discoveredTools map[string]mcp.ToolInfo, cfg *config.Config) {
+func startSlackClient(ctx context.Context, logger *logging.Logger, mcpClients map[string]*mcp.Client, discoveredTools map[string]mcp.ToolInfo, cfg *config.Config) {
 	logger.Info("Starting Slack client...")
 
 	// Initialize RAG client if enabled and add tools to discoveredTools
@@ -652,21 +652,52 @@ func startSlackClient(logger *logging.Logger, mcpClients map[string]*mcp.Client,
 		logger.Fatal("Failed to initialize Slack client: %v", err)
 	}
 
+	// Create a channel to signal when Slack client exits
+	slackDone := make(chan error, 1)
+
 	// Start listening for Slack events in a separate goroutine
 	go func() {
+		defer close(slackDone)
 		if err := client.Run(); err != nil {
-			logger.Fatal("Slack client error: %v", err)
+			logger.ErrorKV("Slack client error", "error", err)
+			slackDone <- err
 		}
 	}()
 
-	logger.Info("Slack MCP Client is now running. Press Ctrl+C to exit.")
+	logger.Info("Slack MCP Client is now running. Waiting for shutdown signal...")
 
-	// Wait for termination signal
+	// Wait for termination signal or context cancellation
 	sigChan := make(chan os.Signal, 1)
 	signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
-	sig := <-sigChan
+	defer signal.Stop(sigChan)
+
+	select {
+	case sig := <-sigChan:
+		logger.Info("Received signal %v, shutting down...", sig)
+	case <-ctx.Done():
+		logger.Info("Context cancelled, shutting down...")
+	case err := <-slackDone:
+		if err != nil {
+			logger.ErrorKV("Slack client exited with error", "error", err)
+		} else {
+			logger.Info("Slack client exited normally")
+		}
+		return // Exit the function if Slack client stopped
+	}
 
-	logger.Info("Received signal %v, shutting down...", sig)
+	// Try to close Slack client gracefully (if Close method is available)
+	logger.Info("Stopping Slack client...")
+	if closeErr := client.Close(); closeErr != nil {
+		logger.ErrorKV("Failed to close Slack client gracefully", "error", closeErr)
+	}
+
+	// Wait for Slack client goroutine to finish with a timeout
+	select {
+	case <-slackDone:
+		logger.Info("Slack client stopped")
+	case <-time.After(5 * time.Second):
+		logger.Warn("Slack client stop timed out")
+	}
 
 	// Gracefully close all MCP clients
 	logger.Info("Closing all MCP clients...")

internal/app/lifecycle.go

@@ -13,7 +13,7 @@ import (
 
 const (
 	minReloadInterval      = 10 * time.Second
-	defaultShutdownTimeout = 10 * time.Second
+	defaultShutdownTimeout = 60 * time.Second
 )
 
 // ReloadTrigger represents the type of trigger that caused a reload
@@ -23,15 +23,15 @@ type ReloadTrigger struct {
 }
 
 // RunWithReload wraps the main application function with reload capability
-func RunWithReload(logger *logging.Logger, configFile string, appFunc func(*logging.Logger) error) error {
+func RunWithReload(logger *logging.Logger, configFile string, appFunc func(context.Context, *logging.Logger) error) error {
 	for {
 		reloadStartTime := time.Now()
 
 		// Load and validate configuration
 		reloadInterval, shouldReload, err := loadAndValidateReloadConfig(configFile, logger)
 		if err != nil || !shouldReload {
 			// Either config loading failed or reload is disabled - run normally
-			return appFunc(logger)
+			return appFunc(context.Background(), logger)
 		}
 
 		logger.InfoKV("Reload enabled", "interval", reloadInterval)
@@ -43,7 +43,7 @@ func RunWithReload(logger *logging.Logger, configFile string, appFunc func(*logg
 		appDone := make(chan error, 1)
 		go func() {
 			// Pass context to application function for graceful shutdown
-			appDone <- runAppWithContext(appCtx, logger, appFunc)
+			appDone <- appFunc(appCtx, logger)
 		}()
 
 		// Wait for reload trigger or app completion
@@ -68,7 +68,8 @@ func RunWithReload(logger *logging.Logger, configFile string, appFunc func(*logg
 				case <-appDone:
 					logger.Info("Application shutdown completed")
 				case <-time.After(defaultShutdownTimeout):
-					logger.WarnKV("Application shutdown timed out", "timeout", defaultShutdownTimeout)
+					logger.WarnKV("Application shutdown timed out, terminating gracefully", "timeout", defaultShutdownTimeout)
+					os.Exit(1)
 				}
 				return nil
 			}
@@ -83,7 +84,8 @@ func RunWithReload(logger *logging.Logger, configFile string, appFunc func(*logg
 			case <-appDone:
 				logger.Info("Current application instance shut down, reinitializing...")
 			case <-time.After(defaultShutdownTimeout):
-				logger.WarnKV("Application shutdown timed out, forcing restart", "timeout", defaultShutdownTimeout)
+				logger.WarnKV("Application shutdown timed out, terminating gracefully", "timeout", defaultShutdownTimeout)
+				return fmt.Errorf("application shutdown timeout after %s", defaultShutdownTimeout)
 			}
 
 			// Record reload metrics
@@ -94,15 +96,6 @@ func RunWithReload(logger *logging.Logger, configFile string, appFunc func(*logg
 	}
 }
 
-// runAppWithContext runs the application function with context for graceful shutdown
-// Note: The context is available but not yet fully integrated with the application.
-// Future enhancement: Modify application functions to accept context for proper cancellation.
-func runAppWithContext(ctx context.Context, logger *logging.Logger, appFunc func(*logging.Logger) error) error {
-	// Run the application function
-	// The context could be used in the future for cancellation signals
-	return appFunc(logger)
-}
-
 // awaitReloadTrigger waits for a reload trigger
 func awaitReloadTrigger(logger *logging.Logger, interval time.Duration) ReloadTrigger {
 	// Setup signal channels with proper cleanup

internal/app/lifecycle_test.go

@@ -96,7 +96,7 @@ func TestConstants(t *testing.T) {
 		t.Errorf("minReloadInterval = %v, expected 10s", minReloadInterval)
 	}
 
-	if defaultShutdownTimeout != 10*time.Second {
-		t.Errorf("defaultShutdownTimeout = %v, expected 10s", defaultShutdownTimeout)
+	if defaultShutdownTimeout != 60*time.Second {
+		t.Errorf("defaultShutdownTimeout = %v, expected 60s", defaultShutdownTimeout)
 	}
 }

internal/slack/client.go

@@ -211,6 +211,14 @@ func (c *Client) Run() error {
 	return c.userFrontend.Run()
 }
 
+// Close gracefully closes the Slack client
+func (c *Client) Close() error {
+	c.logger.Info("Closing Slack client...")
+	// Note: socketmode.Client doesn't have a public Close method
+	// The client will stop when the context is cancelled or when there's a connection error
+	return nil
+}
+
 // handleEvents listens for incoming events and dispatches them.
 func (c *Client) handleEvents() {
 	for evt := range c.userFrontend.GetEventChannel() {